svn commit: r267760 - head/sys/kern

[Mateusz Guzik]

On Mon, Jun 23, 2014 at 11:18:23AM +0300, Konstantin Belousov wrote:
> On Mon, Jun 23, 2014 at 10:05:01AM +0200, Mateusz Guzik wrote:
> > On Mon, Jun 23, 2014 at 10:25:19AM +0300, Konstantin Belousov wrote:
> > > On Mon, Jun 23, 2014 at 09:06:52AM +0200, Mateusz Guzik wrote:
> > > > The table is modified in these functions and is reachable from the rest
> > > > of the kernel (can be found by e.g. sysctl_kern_proc_filedesc), thus
> > > > XLOCK is needed to ensure consistency for readers. It can also be
> > > > altered by mountcheckdirs, although not in a way which disrupts any of
> > > > these functions.
> > > I would think that such cases should be avoided by testing for P_INEXEC,
> > > but I do not insist on this.
> > > 
> > 
> > proc lock has to be dropped before filedesc lock is taken and I don't
> > see any way to prevent the proc from transitioning to P_INEXEC afterwards.
> > Since sysctl_kern_proc_filedesc et al can take a long time it does not
> > seem feasible to pursue this.
> > 

After a second look this problem has to be dealt with.

If traversal while transition to P_INEXEC is allowed, execve dealing
with a setuid binary is problematic. This is more of hypothetical nature,
but with sufficienly long delay it could finish the syscall and start
opening some files, which paths would now be visible for an unprivileged
reader.

That said, I propose adding a counter to struct proc which would which
would block execve. It would be quite similar to p_lock.

iow execve would:

        PROC_LOCK(p);
	p->p_flag |= P_INEXEC; 
        while (p->p_execlock > 0)
                msleep(&p->p_execlock, &p->p_mtx, PWAIT, "execlock", 0);
	PROC_UNLOCK(p);

And it would be mandatory for external fdp consumers to grab the counter.

I'm tempted to add P_GETPIN which would both increase p_lock and p_execlock,
that way the process is guaranteed not to exit and not to execve even
after proc lock is dropped.

There is a separate question if p_execlock should be renamed and
extended to also block any kind of credential changes.

Then the guarantee is even stronger since we know that credentials we
checked against are not going to change for the duration of our
operations, but it is unclear if we need this.

-- 
Mateusz Guzik <mjguzik gmail.com>