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

From: Al Viro
Date: Thu Oct 25 2012 - 08:09:45 EST


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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/