Re: [PATCH] pidhash: Kill switch_exec_pids

[ebiederm@xmission.com (Eric W. Biederman)]

Oleg Nesterov <oleg@tv-sign.ru> writes:

> "Eric W. Biederman" wrote:
>> 
>> 
>> 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.

No problem.  If de_thread was simple and obviously correct
we wouldn't be fixing it :)

>> 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.

Ok.  I thought there was a RCU component to the readers.  I just
lost track of where it was.

> 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.

I hope so.

> Andrew, please drop this one:
>
> 	dont-touch-current-tasks-in-de_thread.patch
>
> Eric's patch includes this cleanup.

I just tested this path against 2.6.16-rc1-mm5 
and the patch applies with just a little fuzz.

Eric