Date: Tue, 2 Feb 2016 18:56:52 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Konstantin Belousov <kib@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: [PATCH 2/2] fork: plug a use after free of the returned process pointer Message-ID: <20160202175652.GA9812@dft-labs.eu> In-Reply-To: <20160202132322.GU91220@kib.kiev.ua> References: <20160201103632.GL91220@kib.kiev.ua> <1454386069-29657-1-git-send-email-mjguzik@gmail.com> <1454386069-29657-3-git-send-email-mjguzik@gmail.com> <20160202132322.GU91220@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Feb 02, 2016 at 03:23:22PM +0200, Konstantin Belousov wrote: > On Tue, Feb 02, 2016 at 05:07:49AM +0100, Mateusz Guzik wrote: > > + flags = fr->fr_flags; > Why not use fr->fr_flags directly ? It is slightly more churn, but IMO > it is worth it. > I'm indiffernet on this one, can change it no problem. > > + /* > > + * Hold the process so that it cannot exit after we make it runnable, > > + * but before we wait for the debugger. > Is this possible ? The forked child must execute through fork_return(), > and there we do ptracestop() before the child has a chance to ever return > to usermode. > > Do you mean a scenario where the debugger detaches before child executes > fork_return() and TDP_STOPATFORK is cleared in advance ? > The comment is somewhat misworded and I forgot to update it, how about just stating we hold the process so that we can mark the thread runnable and not have it disappear under we are done. While I have not tested this particular bug, prior to the patch the following should possible: p2 is untraced and td2 is marked as runnable, after which it exits and p2 is automatically reaped. If the code reaches the TDB_STOPATFORK check after that, PROC_LOCK(p2) succeeds due to processes being type stable. td2 dereference can cause no issues due to threads being type stable as well. But the thread could have been resued in a traced process, thus inducing cv_wait(&p2->p_dbgwait, ..) even though td2 is not linked in p2 anymore and p2 is not even a valid process, making curthread wait indefinitely since there is nobody to wake it up. -- Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160202175652.GA9812>