Messages in this thread |  | | | Date | Fri, 03 Feb 2006 16:26:43 +0300 | | From | Oleg Nesterov <> | | Subject | Re: [PATCH] pidhash: Kill switch_exec_pids |
| |
"Eric W. Biederman" wrote: > > Oleg Nesterov <oleg@tv-sign.ru> writes: > > > Eric W. Biederman wrote: > >> > >> + detach_pid(current, PIDTYPE_PID); > >> + current->pid = leader->pid; > >> + attach_pid(current, PIDTYPE_PID, current->pid); > > > > What happens after de_thread() unlocks tasklist_lock and before > > it is taken again in release_task() ? > > > > In that window find_task_by_pid() will return dead leader, not > > the new leader of thread group. This means we can miss tkill() > > or ptrace(), for example. > > All I have done is enlarged the window where this > race is possible. So for tkill I am not concerned, > as it wants a particular thread. Nor am I concerned > about anything else that wants a particular thread.
Yes, you are right, sorry for noise. We have exactly same situation before de_thread() locks tasklist after killing the leader.
> The fact that the group_leader does not point > at the actual thread group leader might be a problem, > as I have opened a window where that is now the case. > > For signals that is not a problem as signals are still shared. > This applies to most other resources as well.
Actually, now I think this patch fixes a small theoretical bug. Currently we have a tiny window in switch_exec_pids() when it detaches ->pid from PIDTYPE_PID namespace. RCU based kill_proc_info() does not take tasklist, so we can miss a signal.
I have added Paul to the CC: list.
> So until we spot that case I'm ready to put this down > of one of those cases in de_thread that looks wrong > but happens to work. Now if there is a way to make > it work more cleanly that may be worth looking at.
I think you are right.
Andrew, please drop this one:
dont-touch-current-tasks-in-de_thread.patch
Eric's patch includes this cleanup.
Oleg. - 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/
|  |