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:09:53 +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 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.
--
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/