Re: [PATCH 2/2] fix kill_proc_info() vs fork() theoretical race

[Paul E. McKenney]

On Thu, Feb 16, 2006 at 11:56:12PM +0300, Oleg Nesterov wrote:
> "Paul E. McKenney" wrote:
> > 
> > On Wed, Feb 15, 2006 at 10:13:26PM +0300, Oleg Nesterov wrote:
> > > copy_process:
> > >
> > >       attach_pid(p, PIDTYPE_PID, p->pid);
> > >       attach_pid(p, PIDTYPE_TGID, p->tgid);
> > >
> > > What if kill_proc_info(p->pid) happens in between?
> > 
> > Doesn't your patch 1/2 that expanded the scope of siglock in
> > copy_process() prevent this from happening?
> 
> I think, no. Please see below,
> 
> > o       A new process is being created on CPU 0, and does the first
> >         attach_pid() in copy_process(), but has not yet done
> >         the second attach_pid().
> > 
> > o       Meanwhile, on CPU 1, kill_proc_info() successfully looks up the
> >         new process via find_task_by_pid().
> > 
> > o       Also on CPU 1, kill_proc_info() calls group_send_sig_info(),
> >         which checks permissions, locates the sighand structure,
> >         then attempts to acquire siglock.
> 
> ... and takes it. Without CLONE_THREAD (more precisely, CLONE_SIGHAND)
> we have different ->sighand for parent (current) and for the new child.
> 
> copy_process() holds parents's ->sighand, while group_send_sig_info()
> takes child's.

Good point!!!

The other thing to think through is tkill on a thread/process while it
is being created.  I believe that this is OK, since thread-specific
kill must target a specific thread, so does not do the traversal.

Does this match your understanding?

						Thanx, Paul
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/