Mailing List Archive: do_wait: Add missing tasklist unlocking in error paths.

do_wait: Add missing tasklist unlocking in error paths.

Sep 20, 2013, 9:37 AM

Post #1 of 3 (81 views)

It looks like both 64a16caf5, and 98abed020 both introduced error paths to do_wait
where we miss unlocking the tasklist.

Spotted with coverity.

Signed-off-by: Dave Jones <davej@fedoraproject.org>

diff --git a/kernel/exit.c b/kernel/exit.c
index a949819..27004a6 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1526,13 +1526,15 @@ 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;
} while_each_thread(current, tsk);
--
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/

Re: do_wait: Add missing tasklist unlocking in error paths. [ In reply to ]

oleg at redhat

Sep 20, 2013, 9:41 AM

Post #2 of 3 (81 views)

Permalink

On 09/20, Dave Jones wrote:
>
> It looks like both 64a16caf5, and 98abed020 both introduced error paths to do_wait
> where we miss unlocking the tasklist.
>
> Spotted with coverity.

Not really, afaics.

> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1526,13 +1526,15 @@ repeat:
> tsk = current;
> do {
> retval = do_wait_thread(wo, tsk);
> - if (retval)
> + if (retval) {
> + read_unlock(&tasklist_lock);

note that do_wait_thread() paths should drop tasklist if it returns non-zero.

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/

Re: do_wait: Add missing tasklist unlocking in error paths. [ In reply to ]

davej at redhat

Sep 20, 2013, 10:03 AM

Post #3 of 3 (82 views)

Permalink

On Fri, Sep 20, 2013 at 06:41:44PM +0200, Oleg Nesterov wrote:
> On 09/20, Dave Jones wrote:
> >
> > It looks like both 64a16caf5, and 98abed020 both introduced error paths to do_wait
> > where we miss unlocking the tasklist.
> >
> > Spotted with coverity.
>
> Not really, afaics.
>
> > --- a/kernel/exit.c
> > +++ b/kernel/exit.c
> > @@ -1526,13 +1526,15 @@ repeat:
> > tsk = current;
> > do {
> > retval = do_wait_thread(wo, tsk);
> > - if (retval)
> > + if (retval) {
> > + read_unlock(&tasklist_lock);
>
> note that do_wait_thread() paths should drop tasklist if it returns non-zero.

Ah, I missed the unlock in wait_task_continued.

I'm not sure why the checker didn't infer that. Perhaps it thinks it's possible we
can get take one of the early returns in that function before we do the tasklist unlock.

thanks,

Dave

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