Date: Fri, 11 Jul 2014 04:43:51 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik <mjg@FreeBSD.org> Subject: Re: svn commit: r267760 - head/sys/kern Message-ID: <20140711024351.GA18214@dft-labs.eu> In-Reply-To: <20140623163523.GK93733@kib.kiev.ua> References: <201406230128.s5N1SIYK097224@svn.freebsd.org> <20140623064044.GD93733@kib.kiev.ua> <20140623070652.GA27040@dft-labs.eu> <20140623072519.GE93733@kib.kiev.ua> <20140623080501.GB27040@dft-labs.eu> <20140623081823.GG93733@kib.kiev.ua> <20140623131653.GC27040@dft-labs.eu> <20140623163523.GK93733@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Jun 23, 2014 at 07:35:23PM +0300, Konstantin Belousov wrote: > On Mon, Jun 23, 2014 at 03:16:53PM +0200, Mateusz Guzik wrote: > > 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. > I thought about this too. In fact, I considered using PHOLD for this. > > > > > 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. > See above about PHOLD. > > > > > 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. > > If doing separate execlock/p_lock, I think that it could be possible > to use per-process sx lock instead of hand-rolling the counter. The > accessors would lock sx shared, while kern_execve would take it in > exclusive mode. Both patches need some cleaning up. The name 'keeplock' is no exactly the best either. In both cases the same mechanism blocks both exec and exit, this can be split if needed (p_lock would still cover exit, p_something would cover exec). Here is a version with sx lock: http://people.freebsd.org/~mjg/patches/exec-exit-hold-wait.patch I'm not really happy with this. Reading foreign fdt is very rare and this adds lock + unlock for every exec and exit. On the other hand mere counter version is rather simple: http://people.freebsd.org/~mjg/patches/exec-exit-hold-nolock.patch I don't have strong opinion here, but prefer the latter. -- Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140711024351.GA18214>