[PATCH 2/2] fork: plug a use after free of the returned process pointer

[Mateusz Guzik]

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>