Messages in this thread | | | Date | Thu, 16 Feb 2006 11:53:41 -0800 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH 2/2] fix kill_proc_info() vs fork() theoretical race |
| |
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |