linux-kernel - Re: [PATCH] exec: do not leave bprm->interp on stack

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]

Date:	Thu, 25 Oct 2012 13:38:44 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	P J P <ppandit@...hat.com>
Cc:	Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Josh Triplett <josh@...htriplett.org>,
	Serge Hallyn <serge.hallyn@...onical.com>,
	linux-fsdevel@...r.kernel.org, halfdog <me@...fdog.net>
Subject: Re: [PATCH] exec: do not leave bprm->interp on stack

On Thu, Oct 25, 2012 at 01:09:53PM +0100, Al Viro wrote:
> On Thu, Oct 25, 2012 at 05:16:22PM +0530, P J P wrote:
> > 
> >   Hello Kees,
> > 
> > +-- On Wed, 24 Oct 2012, Kees Cook wrote --+
> > | What should the code here _actually_ be doing? The _script and _misc 
> > | handlers expect to rewrite the bprm contents and recurse, but the module 
> > | loader want to try again. It's not clear to me what the binfmt module 
> > | handler is even there for; I don't see any binfmt-XXXX aliases in the tree. 
> > | If nothing uses it, should we just rip it out? That would solve it too.
> 
> ; grep binfmt- /etc/*/* 2>/dev/null 
> /etc/modprobe.d/aliases.conf:install binfmt-0000 /bin/true
> /etc/modprobe.d/aliases.conf:alias binfmt-204 binfmt_aout
> /etc/modprobe.d/aliases.conf:alias binfmt-263 binfmt_aout
> /etc/modprobe.d/aliases.conf:alias binfmt-264 binfmt_aout
> /etc/modprobe.d/aliases.conf:alias binfmt-267 binfmt_aout
> /etc/modprobe.d/aliases.conf:alias binfmt-387 binfmt_aout
> ; dpkg -S /etc/modprobe.d/aliases.conf 
> module-init-tools: /etc/modprobe.d/aliases.conf
> 
> > I've been following this issue and updated versions of HDs patch. Below is a 
> > small patch to search_binary_handler() routine, which attempts to make the 
> > request_module call before calling load_script routine.
> > 
> > Besides fixing the stack disclosure issue it also helps to *simplify* the 
> > search_binary_handler routine by removing the -for (try=0;try<2;try++)- loop.
> > 
> > I'd really appreciate any comments/suggestions you may have.
> 
> Suggestion: try testing your patches once in a while.  Stopping to think
> for a minute would also help - you've turned every execve() into "do
> request_module() first".  How do you suppose request_module() works?  And
> how would modprobe be able to run?  IOW, this request_module() will be
> stopped by protection against infinite loops, at which point execve will
> proceed with already present binfmt, without having loaded anything.
> But that's even worse than slowdown on each execve (with a lot of whining
> in process), because *every* request_module() will fail now due to the same
> loop prevention.

... and after the second look at your patch, looks like another breakage
in there will have a different effect - it doesn't just eliminate the
first pass through the loop, it inverts the test for "should I try
request_module()".  Overall result is a bit less painful - request_module()
isn't broken on loop prevention, but
	* every bleeding script will have bogus execution of modprobe done
at execve time (and you'd better pray that /sbin/modprobe isn't a shell
script wrapper around the actual binary, or you *will* get loop prevention
kick in)
	* none of the existing binfmt-<...> aliases is going to be hit
now; IOW, all usecases got broken.  Granted, realistically it just means
broken modular aout support, but then it's the only reason to have that
request_module() there in the first place.
--
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/