Date: Sun, 5 Oct 2014 20:46:21 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-hackers@freebsd.org Subject: Re: fork: hold newly created processes Message-ID: <20141005184620.GC9262@dft-labs.eu> In-Reply-To: <20141005171457.GA26076@kib.kiev.ua> References: <20141005102912.GB9262@dft-labs.eu> <20141005171457.GA26076@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Oct 05, 2014 at 08:14:58PM +0300, Konstantin Belousov wrote: > On Sun, Oct 05, 2014 at 12:29:12PM +0200, Mateusz Guzik wrote: > > fork: hold newly created processes > > > > Consumers of fork1 -> do_fork receive new proc pointer, but nothing > > guarnatees its stability at that time. > > > > New process could already exit and be waited for, in which case we get a > > use after free. > Since the new process is the child of the current process, it can happen > only if the code is self-inflicting. I can imagine that the only way > to achieve it, do wait() in other thread. > Yes, the patch in question is an anti local dos measure. > That said, there is no harm for the kernel state, since struct proc is > type-stable, so we do not dereference a random memory, do you agree ? > We could return non-existing or reused pid, but this can occur with > your patch as well, since the same exit/wait could be done while forking > thread executes syscall return code. Pinning the process with PHOLD means *fork will always return the right pid. Of course the child could be gone by the time fork returns, but this is not a problem. In fork1 you can find: do_fork(td, flags, newproc, td2, vm2, pdflags); /* * Return child proc pointer to parent. */ *procp = newproc; if (flags & RFPROCDESC) { procdesc_finit(newproc->p_procdesc, fp_procdesc); fdrop(fp_procdesc, td); } racct_proc_fork_done(newproc); return (0); Here nothing guarantees newproc is stable and I managed to provoke a crash with null pointer dereference in procdesc_finit since it got a now cleared up process. I think it is possible it will get a different process, provided someone managed to fork it in the meantime. Also, although I didn't try to provoke anything, linux emulation layer does a lot of work with newly returned proc pointer. -- Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141005184620.GC9262>