lkml.org 
[lkml]   [2012]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3] kmod: Avoid deadlock by recursive kmod call.
From
Date
("[PATCH v3] kmod: Avoid deadlock by recursive kmod call." didn't arrive at ML
due to mail delivery failure. The V3 patch can be found at
https://bugzilla.kernel.org/show_bug.cgi?id=42579 )

Andrew Morton wrote:
> I suppose we should fix this, although it's obscure.
>
> The changelog doesn't tell people how the bug might occur, which is
> bad. I suggest adding this text to it:
>
> : This bug was observed when using a corrupted /sbin/hotplug binary. The
> : corrupted binary caused a call to request_module("binfmt-0000").
> : search_binary_handler() uses UMH_WAIT_EXEC.

A corrupted /sbin/hotplug binary is just a simplest example. There are various
hooks called during do_execve() operation (e.g. security_bprm_check(),
audit_bprm(), "struct linux_binfmt"->load_binary()). If one of such hooks
triggers (in the future, but maybe already) UMH_WAIT_EXEC, this deadlock
will happen even if /sbin/hotplug is not corrupted.

> I prefer patch A - all that poking around at stack slots is hacky.
OK.

> > --- linux-3.2.orig/fs/exec.c
> > +++ linux-3.2/fs/exec.c
> > @@ -1406,6 +1406,7 @@ int search_binary_handler(struct linux_b
> > fput(bprm->file);
> > bprm->file = NULL;
> > current->did_exec = 1;
> > + current->kmod_thread = 0;
> > proc_exec_connector(current);
> > return retval;
> > }
>
> Special-casing this assignment down in this particular client of kmod
> looks bad. We need to find somewhere else to do this. Perferably
> within the kmod code itself, or possibly over in exec.c or fork.c.
>
Well, kmod code is no longer called if do_execve() succeeded.

In "[PATCH v2] kmod: Avoid deadlock by recursive kmod call"
( https://lkml.org/lkml/2012/1/4/111 ), I forgot to do

current->kmod_thread = 0;

when do_execve() succeeded. Not clearing this flag might overkill functionality
of /sbin/hotplug and its descendants. If we want to clear this flag, I think we
cannot help doing it in fork.c or exec.c (or do like patch B which does not
need this flag).


\
 
 \ /
  Last update: 2012-01-19 03:11    [W:1.516 / U:0.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site