On 21 May 2015 12:56:22 CEST, Ingo Molnar <mi...@kernel.org> wrote: > >* Frans Klaver <franskla...@gmail.com> wrote: > >> On Thu, May 21, 2015 at 8:03 AM, Ingo Molnar <mi...@kernel.org> >wrote: >> > >> > * Maninder Singh <maninder...@samsung.com> wrote: >> > >> >> EP-F6AA0618C49C4AEDA73BFF1B39950BAB >> >> Hi, >> >> >> >> From: Maninder Singh <maninder...@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 <maninder...@samsung.com> >> >> Signed-off-by: Vaneet Narang <v.nar...@samsung.com> >> >> Reviewd-by: Akhilesh Kumar <akhiles...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/