[EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[Maninder Singh]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=windows-1252, Size: 1144 bytes --]

 EP-F6AA0618C49C4AEDA73BFF1B39950BAB
Hi,

From: Maninder Singh <maninder1.s@samsung.com>

Subject: [PATCH 1/1] kernel/exit.c : Fix missing task_unlock

This patch adds missing read_unlock if do_wait_thread or ptrace_do_wait 
returns non zero.

Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
Signed-off-by: Vaneet Narang <v.narang@samsung.com>
Reviewd-by: Akhilesh Kumar <akhilesh.k@samsung.com>
---
 kernel/exit.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 22fcc05..31a061f 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1486,12 +1486,16 @@ repeat:
 	tsk = current;
 	do {
 		retval = do_wait_thread(wo, tsk);
-		if (retval)
+		if (retval) {
+			read_unlock(&tasklist_lock);
 			goto end;
+		}
 
 		retval = ptrace_do_wait(wo, tsk);
-		if (retval)
+		if (retval) {
+			read_unlock(&tasklist_lock);
 			goto end;
+		}
 
 		if (wo->wo_flags & __WNOTHREAD)
 			break;
-- 
1.7.1

Thanks 
Maninder Singhÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[Ingo Molnar]

* Maninder Singh <maninder1.s@samsung.com> wrote:

>  EP-F6AA0618C49C4AEDA73BFF1B39950BAB
> Hi,
> 
> From: Maninder Singh <maninder1.s@samsung.com>
> 
> Subject: [PATCH 1/1] kernel/exit.c : Fix missing task_unlock
> 
> This patch adds missing read_unlock if do_wait_thread or ptrace_do_wait 
> returns non zero.
> 
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
> Reviewd-by: Akhilesh Kumar <akhilesh.k@samsung.com>
> ---
>  kernel/exit.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/exit.c b/kernel/exit.c
> index 22fcc05..31a061f 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1486,12 +1486,16 @@ repeat:
>  	tsk = current;
>  	do {
>  		retval = do_wait_thread(wo, tsk);
> -		if (retval)
> +		if (retval) {
> +			read_unlock(&tasklist_lock);
>  			goto end;
> +		}
>  
>  		retval = ptrace_do_wait(wo, tsk);
> -		if (retval)
> +		if (retval) {
> +			read_unlock(&tasklist_lock);
>  			goto end;
> +		}
>  
>  		if (wo->wo_flags & __WNOTHREAD)
>  			break;

That's surprising and the changelog is lacking.

So the last time that code was touched upstream was 7 years ago:

 commit 64a16caf5e3417ee32f670debcb5857b02a9e08e
 Author: Oleg Nesterov <oleg@redhat.com>
 Date:   Wed Jun 17 16:27:40 2009 -0700

    do_wait: simplify retval/tsk_result/notask_error mess

please explain whether what you fix is:

  1) an ancient bug that somehow nobody ever triggered (plus analysis 
     of why it wasn't triggered)

  2) a new bug introduced by commit XYZ (plus analysis)

  3) something else

Thanks,

	Ingo

Re: [EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[Frans Klaver]

On Thu, May 21, 2015 at 8:03 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Maninder Singh <maninder1.s@samsung.com> wrote:
>
>>  EP-F6AA0618C49C4AEDA73BFF1B39950BAB
>> Hi,
>>
>> From: Maninder Singh <maninder1.s@samsung.com>
>>
>> Subject: [PATCH 1/1] kernel/exit.c : Fix missing task_unlock
>>
>> This patch adds missing read_unlock if do_wait_thread or ptrace_do_wait
>> returns non zero.
>>
>> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
>> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
>> Reviewd-by: Akhilesh Kumar <akhilesh.k@samsung.com>
>> ---
>>  kernel/exit.c |    8 ++++++--
>>  1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/exit.c b/kernel/exit.c
>> index 22fcc05..31a061f 100644
>> --- a/kernel/exit.c
>> +++ b/kernel/exit.c
>> @@ -1486,12 +1486,16 @@ repeat:
>>       tsk = current;
>>       do {
>>               retval = do_wait_thread(wo, tsk);
>> -             if (retval)
>> +             if (retval) {
>> +                     read_unlock(&tasklist_lock);
>>                       goto end;
>> +             }
>>
>>               retval = ptrace_do_wait(wo, tsk);
>> -             if (retval)
>> +             if (retval) {
>> +                     read_unlock(&tasklist_lock);
>>                       goto end;
>> +             }
>>
>>               if (wo->wo_flags & __WNOTHREAD)
>>                       break;
>
> That's surprising <snip>

Still it looks like it is a legitimate change. I don't see where the
unlock would be done otherwise.

I do wonder if this would look nicer if the whole locked part would be
pulled out into a separate (inline) function. That would render the
repeated read_unlock()s unnecessary and possibly also prevent a
goto/label mess if that were to be attempted in-line.

Frans

Re: [EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[Ingo Molnar]

* Frans Klaver <fransklaver@gmail.com> wrote:

> On Thu, May 21, 2015 at 8:03 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Maninder Singh <maninder1.s@samsung.com> wrote:
> >
> >>  EP-F6AA0618C49C4AEDA73BFF1B39950BAB
> >> Hi,
> >>
> >> From: Maninder Singh <maninder1.s@samsung.com>
> >>
> >> Subject: [PATCH 1/1] kernel/exit.c : Fix missing task_unlock
> >>
> >> This patch adds missing read_unlock if do_wait_thread or ptrace_do_wait
> >> returns non zero.
> >>
> >> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> >> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
> >> Reviewd-by: Akhilesh Kumar <akhilesh.k@samsung.com>
> >> ---
> >>  kernel/exit.c |    8 ++++++--
> >>  1 files changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/kernel/exit.c b/kernel/exit.c
> >> index 22fcc05..31a061f 100644
> >> --- a/kernel/exit.c
> >> +++ b/kernel/exit.c
> >> @@ -1486,12 +1486,16 @@ repeat:
> >>       tsk = current;
> >>       do {
> >>               retval = do_wait_thread(wo, tsk);
> >> -             if (retval)
> >> +             if (retval) {
> >> +                     read_unlock(&tasklist_lock);
> >>                       goto end;
> >> +             }
> >>
> >>               retval = ptrace_do_wait(wo, tsk);
> >> -             if (retval)
> >> +             if (retval) {
> >> +                     read_unlock(&tasklist_lock);
> >>                       goto end;
> >> +             }
> >>
> >>               if (wo->wo_flags & __WNOTHREAD)
> >>                       break;
> >
> > That's surprising <snip>
> 
> Still it looks like it is a legitimate change. I don't see where the 
> unlock would be done otherwise.

No, it does not look like a legitimate change, that's why I asked the 
questions. I think this patch breaks the kernel badly.

As it is explained in the comments as well, the various wait-loop 
functions (do_wait_thread(), ptrace_do_wait()) fundamentally unlock 
the tasklist_lock if they return an error.

NAK.

Thanks,

	Ingo

Re: [EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[Frans Klaver]

On 21 May 2015 12:56:22 CEST, Ingo Molnar <mingo@kernel.org> wrote:
>
>* Frans Klaver <fransklaver@gmail.com> wrote:
>
>> On Thu, May 21, 2015 at 8:03 AM, Ingo Molnar <mingo@kernel.org>
>wrote:
>> >
>> > * Maninder Singh <maninder1.s@samsung.com> wrote:
>> >
>> >>  EP-F6AA0618C49C4AEDA73BFF1B39950BAB
>> >> Hi,
>> >>
>> >> From: Maninder Singh <maninder1.s@samsung.com>
>> >>
>> >> Subject: [PATCH 1/1] kernel/exit.c : Fix missing task_unlock
>> >>
>> >> This patch adds missing read_unlock if do_wait_thread or
>ptrace_do_wait
>> >> returns non zero.
>> >>
>> >> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
>> >> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
>> >> Reviewd-by: Akhilesh Kumar <akhilesh.k@samsung.com>
>> >> ---
>> >>  kernel/exit.c |    8 ++++++--
>> >>  1 files changed, 6 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/kernel/exit.c b/kernel/exit.c
>> >> index 22fcc05..31a061f 100644
>> >> --- a/kernel/exit.c
>> >> +++ b/kernel/exit.c
>> >> @@ -1486,12 +1486,16 @@ repeat:
>> >>       tsk = current;
>> >>       do {
>> >>               retval = do_wait_thread(wo, tsk);
>> >> -             if (retval)
>> >> +             if (retval) {
>> >> +                     read_unlock(&tasklist_lock);
>> >>                       goto end;
>> >> +             }
>> >>
>> >>               retval = ptrace_do_wait(wo, tsk);
>> >> -             if (retval)
>> >> +             if (retval) {
>> >> +                     read_unlock(&tasklist_lock);
>> >>                       goto end;
>> >> +             }
>> >>
>> >>               if (wo->wo_flags & __WNOTHREAD)
>> >>                       break;
>> >
>> > That's surprising <snip>
>>
>> Still it looks like it is a legitimate change. I don't see where the
>> unlock would be done otherwise.
>
>No, it does not look like a legitimate change, that's why I asked the
>questions. I think this patch breaks the kernel badly.
>
>As it is explained in the comments as well, the various wait-loop
>functions (do_wait_thread(), ptrace_do_wait()) fundamentally unlock
>the tasklist_lock if they return an error.

Ah, right. Given that I agree. I can imagine a static checker not
seeing that either. Sorry for the noise here.

Thanks,
Frans

Re: [EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[Oleg Nesterov]

On 05/21, Maninder Singh wrote:
>
>  EP-F6AA0618C49C4AEDA73BFF1B39950BAB
> Hi,
>
> From: Maninder Singh <maninder1.s@samsung.com>
>
> Subject: [PATCH 1/1] kernel/exit.c : Fix missing task_unlock
>
> This patch adds missing read_unlock if do_wait_thread or ptrace_do_wait
> returns non zero.

Confused...

wait_consider_task() should drop tasklist_lock if it returns non-zero?


> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1486,12 +1486,16 @@ repeat:
>  	tsk = current;
>  	do {
>  		retval = do_wait_thread(wo, tsk);
> -		if (retval)
> +		if (retval) {
> +			read_unlock(&tasklist_lock);
>  			goto end;
> +		}
>
>  		retval = ptrace_do_wait(wo, tsk);
> -		if (retval)
> +		if (retval) {
> +			read_unlock(&tasklist_lock);
>  			goto end;
> +		}

Well, the patch is obviously wrong. Because, again, tasklist_lock was
already unlocked if (say) wait_task_zombie() reaps a child.

If you think there is a case which forgets to unlock, please tell us
more.

Oleg.

Re: [EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[Maninder Singh]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=windows-1252, Size: 2119 bytes --]

EP-F6AA0618C49C4AEDA73BFF1B39950BAB
>> Hi,
>> 
>> From: Maninder Singh <maninder1.s@samsung.com>
>> 
>> Subject: [PATCH 1/1] kernel/exit.c : Fix missing task_unlock
>> 
     Subject: [PATCH 1/1] kernel/exit.c : Fix missing read_unlock
     
>> This patch adds missing read_unlock if do_wait_thread or ptrace_do_wait 
>> returns non zero.

      Reported By Prevent Under Missing unlock category(program hangs):-
      missing_unlock: returning without unlocking tasklist_lock
>> 
>> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
>> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
>> Reviewd-by: Akhilesh Kumar <akhilesh.k@samsung.com>
>> ---
>>  kernel/exit.c |    8 ++++++--
>>  1 files changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/kernel/exit.c b/kernel/exit.c
>> index 22fcc05..31a061f 100644
>> --- a/kernel/exit.c
>> +++ b/kernel/exit.c
>> @@ -1486,12 +1486,16 @@ repeat:
>>  	tsk = current;
>>  	do {
>>  		retval = do_wait_thread(wo, tsk);
>> -		if (retval)
>> +		if (retval) {
>> +			read_unlock(&tasklist_lock);
>>  			goto end;
>> +		}
>>  
>>  		retval = ptrace_do_wait(wo, tsk);
>> -		if (retval)
>> +		if (retval) {
>> +			read_unlock(&tasklist_lock);
>>  			goto end;
>> +		}
>>  
>>  		if (wo->wo_flags & __WNOTHREAD)
>>  			break;
>
>That's surprising and the changelog is lacking.

>So the last time that code was touched upstream was 7 years ago:

> commit 64a16caf5e3417ee32f670debcb5857b02a9e08e
> Author: Oleg Nesterov <oleg@redhat.com>
> Date:   Wed Jun 17 16:27:40 2009 -0700

>    do_wait: simplify retval/tsk_result/notask_error mess

>please explain whether what you fix is:

>  1) an ancient bug that somehow nobody ever triggered (plus analysis 
>     of why it wasn't triggered)

>  2) a new bug introduced by commit XYZ (plus analysis)

>  3) something else

This issue is reported by Prevent Under category Missing Unlock, So we think it should be reported to maintainers.

>Thanks,
>	Ingo

Thanks.ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[Ingo Molnar]

* Maninder Singh <maninder1.s@samsung.com> wrote:

> EP-F6AA0618C49C4AEDA73BFF1B39950BAB
> >> Hi,
> >> 
> >> From: Maninder Singh <maninder1.s@samsung.com>
> >> 
> >> Subject: [PATCH 1/1] kernel/exit.c : Fix missing task_unlock
> >> 
>      Subject: [PATCH 1/1] kernel/exit.c : Fix missing read_unlock
>      
> >> This patch adds missing read_unlock if do_wait_thread or ptrace_do_wait 
> >> returns non zero.
> 
>       Reported By Prevent Under Missing unlock category(program hangs):-
>       missing_unlock: returning without unlocking tasklist_lock
> >> 
> >> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> >> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
> >> Reviewd-by: Akhilesh Kumar <akhilesh.k@samsung.com>
> >> ---
> >>  kernel/exit.c |    8 ++++++--
> >>  1 files changed, 6 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/kernel/exit.c b/kernel/exit.c
> >> index 22fcc05..31a061f 100644
> >> --- a/kernel/exit.c
> >> +++ b/kernel/exit.c
> >> @@ -1486,12 +1486,16 @@ repeat:
> >>  	tsk = current;
> >>  	do {
> >>  		retval = do_wait_thread(wo, tsk);
> >> -		if (retval)
> >> +		if (retval) {
> >> +			read_unlock(&tasklist_lock);
> >>  			goto end;
> >> +		}
> >>  
> >>  		retval = ptrace_do_wait(wo, tsk);
> >> -		if (retval)
> >> +		if (retval) {
> >> +			read_unlock(&tasklist_lock);
> >>  			goto end;
> >> +		}
> >>  
> >>  		if (wo->wo_flags & __WNOTHREAD)
> >>  			break;
> >
> >That's surprising and the changelog is lacking.
> 
> >So the last time that code was touched upstream was 7 years ago:
> 
> > commit 64a16caf5e3417ee32f670debcb5857b02a9e08e
> > Author: Oleg Nesterov <oleg@redhat.com>
> > Date:   Wed Jun 17 16:27:40 2009 -0700
> 
> >    do_wait: simplify retval/tsk_result/notask_error mess
> 
> >please explain whether what you fix is:
> 
> >  1) an ancient bug that somehow nobody ever triggered (plus analysis 
> >     of why it wasn't triggered)
> 
> >  2) a new bug introduced by commit XYZ (plus analysis)
> 
> >  3) something else
> 
> This issue is reported by Prevent Under category Missing Unlock, So 
> we think it should be reported to maintainers.

Huh? In what way does your reply answer my questions?

Your patch is breaking the kernel, and badly so.

Thanks,

	Ingo

Re: [EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[Maninder Singh]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=windows-1252, Size: 2666 bytes --]

EP-F6AA0618C49C4AEDA73BFF1B39950BAB

Hi Ingo,

>> >> Hi,
>> >> 
>> >> From: Maninder Singh <maninder1.s@samsung.com>
>> >> 
>> >> Subject: [PATCH 1/1] kernel/exit.c : Fix missing task_unlock
>> >> 
>>      Subject: [PATCH 1/1] kernel/exit.c : Fix missing read_unlock
>>      
>> >> This patch adds missing read_unlock if do_wait_thread or ptrace_do_wait 
>> >> returns non zero.
>> 
>>       Reported By Prevent Under Missing unlock category(program hangs):-
>>       missing_unlock: returning without unlocking tasklist_lock
>> >> 
>> >> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
>> >> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
>> >> Reviewd-by: Akhilesh Kumar <akhilesh.k@samsung.com>
>> >> ---
>> >>  kernel/exit.c |    8 ++++++--
>> >>  1 files changed, 6 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/kernel/exit.c b/kernel/exit.c
>> >> index 22fcc05..31a061f 100644
>> >> --- a/kernel/exit.c
>> >> +++ b/kernel/exit.c
>> >> @@ -1486,12 +1486,16 @@ repeat:
>> >>  	tsk = current;
>> >>  	do {
>> >>  		retval = do_wait_thread(wo, tsk);
>> >> -		if (retval)
>> >> +		if (retval) {
>> >> +			read_unlock(&tasklist_lock);
>> >>  			goto end;
>> >> +		}
>> >>  
>> >>  		retval = ptrace_do_wait(wo, tsk);
>> >> -		if (retval)
>> >> +		if (retval) {
>> >> +			read_unlock(&tasklist_lock);
>> >>  			goto end;
>> >> +		}
>> >>  
>> >>  		if (wo->wo_flags & __WNOTHREAD)
>> >>  			break;
>> >
>> >That's surprising and the changelog is lacking.
>> 
>> >So the last time that code was touched upstream was 7 years ago:
>> 
>> > commit 64a16caf5e3417ee32f670debcb5857b02a9e08e
>> > Author: Oleg Nesterov <oleg@redhat.com>
>> > Date:   Wed Jun 17 16:27:40 2009 -0700
>> 
>> >    do_wait: simplify retval/tsk_result/notask_error mess
>> 
>> >please explain whether what you fix is:
>> 
>> >  1) an ancient bug that somehow nobody ever triggered (plus analysis 
>> >     of why it wasn't triggered)
>> 
>> >  2) a new bug introduced by commit XYZ (plus analysis)
>> 
>> >  3) something else
>> 
>> This issue is reported by Prevent Under category Missing Unlock, So 
>> we think it should be reported to maintainers.

>Huh? In what way does your reply answer my questions?

we sent this fix because we were doing static analysis of kernel code by tool coverity analyzer (Prevent), and it shows this unlock mismatch,
Thats why we think to report this whether it is right or not .

>Your patch is breaking the kernel, and badly so.

Sorry for the noise .

>Thanks,

>	Ingo


Thanksÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [EDT][PATCH] kernel/exit.c : Fix missing read_unlock

[Maninder Singh]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=windows-1252, Size: 1262 bytes --]

 EP-F6AA0618C49C4AEDA73BFF1B39950BAB

Hi Oleg,

>> Hi,
>>
>> From: Maninder Singh <maninder1.s@samsung.com>
>>
>> Subject: [PATCH 1/1] kernel/exit.c : Fix missing task_unlock
>>
>> This patch adds missing read_unlock if do_wait_thread or ptrace_do_wait
>> returns non zero.
>
>Confused...
>
>wait_consider_task() should drop tasklist_lock if it returns non-zero?
>
>
>> --- a/kernel/exit.c
>> +++ b/kernel/exit.c
>> @@ -1486,12 +1486,16 @@ repeat:
>>  	tsk = current;
>>  	do {
>>  		retval = do_wait_thread(wo, tsk);
>> -		if (retval)
>> +		if (retval) {
>> +			read_unlock(&tasklist_lock);
>>  			goto end;
>> +		}
>>
>>  		retval = ptrace_do_wait(wo, tsk);
>> -		if (retval)
>> +		if (retval) {
>> +			read_unlock(&tasklist_lock);
>>  			goto end;
>> +		}
>
>Well, the patch is obviously wrong. Because, again, tasklist_lock was
>already unlocked if (say) wait_task_zombie() reaps a child.

Yes, agree, My wrong
>If you think there is a case which forgets to unlock, please tell us
>more.
I have checked It is getting unlocked in wait_task_zombie Sorry for unnecessary disturbance. 

>Oleg.
Thanks

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥