linux-kernel - Re: [PATCH v3] kmod: Avoid deadlock by recursive kmod call.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201201190207.q0J27Mr5099912@www262.sakura.ne.jp>
Date:	Thu, 19 Jan 2012 11:07:22 +0900
From:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:	akpm@...ux-foundation.org
Cc:	arjan@...ux.intel.com, richard@....at,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] kmod: Avoid deadlock by recursive kmod call.

("[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).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/