linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: P J P <ppandit@redhat.com>
Cc: Kees Cook <keescook@chromium.org>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Josh Triplett <josh@joshtriplett.org>,
	Serge Hallyn <serge.hallyn@canonical.com>,
	linux-fsdevel@vger.kernel.org, halfdog <me@halfdog.net>
Subject: Re: [PATCH] exec: do not leave bprm->interp on stack
Date: Thu, 25 Oct 2012 13:09:53 +0100	[thread overview]
Message-ID: <20121025120952.GI2616@ZenIV.linux.org.uk> (raw)
In-Reply-To: <alpine.LFD.2.02.1210251636480.23155@wniryva.cad.erqung.pbz>

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.

  parent reply	other threads:[~2012-10-25 12:09 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-24 23:20 [PATCH] exec: do not leave bprm->interp on stack Kees Cook
2012-10-25  4:16 ` Al Viro
2012-10-25  6:21   ` Kees Cook
2012-10-25 11:46     ` P J P
2012-10-25 12:03       ` Tetsuo Handa
2012-10-25 12:57         ` P J P
2012-10-25 12:09       ` Al Viro [this message]
2012-10-25 12:38         ` Al Viro
2012-10-26 17:38           ` P J P
2012-10-26 18:36             ` Al Viro
2012-10-27 10:47               ` P J P
2012-10-27 17:05                 ` Kees Cook
2012-10-27 20:16                   ` P J P
2012-10-28  3:32                     ` Kees Cook
2012-11-06  8:10                       ` P J P
2012-11-12 22:10                         ` Kees Cook
2012-11-13  6:50                           ` halfdog
2012-11-16 12:50                           ` P J P
2012-11-16 18:00                             ` Kees Cook
2012-11-18 19:04                               ` P J P
2012-11-18 19:34                                 ` Kees Cook
2012-11-19  6:57                                   ` P J P
2012-11-19 20:41                                     ` Kees Cook
2012-11-20  7:04                                       ` P J P
2012-11-22 20:06                                       ` P J P
2012-11-23 18:43                                       ` P J P
2012-11-23 23:12                                         ` Tetsuo Handa
     [not found]                                       ` <alpine.LFD.2.02.1211221934220.19768@wniryva.cad.erqung.pbz>
     [not found]                                         ` <CAGXu5jL8zCt5ghW4ODPL9+SyC9+mbuw=4JMUngepY5fC8pSikQ@mail.gmail.com>
2012-11-26  7:09                                           ` P J P
     [not found]                                     ` <CA+55aFx3LFH5Xj1OkNoy7vN5w8y5tH39MUDujKqF3BdnmYibLQ@mail.gmail.com>
2012-11-20  7:08                                       ` P J P

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121025120952.GI2616@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=josh@joshtriplett.org \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=me@halfdog.net \
    --cc=ppandit@redhat.com \
    --cc=serge.hallyn@canonical.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).