All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
	Evgeniy Polyakov <zbr@ioremap.net>,
	Kees Cook <keescook@chromium.org>,
	Zach Levis <zml@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 7/8] exec: don't retry if request_module() fails
Date: Mon, 5 Aug 2013 15:41:45 +0200	[thread overview]
Message-ID: <20130805134145.GA15652@redhat.com> (raw)
In-Reply-To: <20130805134113.GA15603@redhat.com>

A separate one-liner for better documentation.

It doesn't make sense to retry if request_module() fails to exec
/sbin/modprobe, add the additional "request_module() < 0" check.

However, this logic still doesn't look exactly right:

1. It would be better to check "request_module() != 0", the user
   space modprobe process should report the correct exit code.
   But I didn't dare to add the user-visible change.

2. The whole ENOEXEC logic looks suboptimal. Suppose that we try
   to exec a "#!path-to-unsupported-binary" script. In this case
   request_module() + "retry" will be done twice: first by the
   "depth == 1" code, and then again by the "depth == 0" caller
   which doesn't make sense.

3. And note that in the case above bprm->buf was already changed
   by load_script()->prepare_binprm(), so this looks even more
   ugly.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Kees Cook <keescook@chromium.org>
---
 fs/exec.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index f69cf54..682895d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1418,7 +1418,8 @@ int search_binary_handler(struct linux_binprm *bprm)
 		if (printable(bprm->buf[0]) && printable(bprm->buf[1]) &&
 		    printable(bprm->buf[2]) && printable(bprm->buf[3]))
 			return retval;
-		request_module("binfmt-%04x", *(ushort *)(bprm->buf + 2));
+		if (request_module("binfmt-%04x", *(ushort *)(bprm->buf + 2)) < 0)
+			return retval;
 		need_retry = false;
 		goto retry;
 	}
-- 
1.5.5.1


  parent reply	other threads:[~2013-08-05 13:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-05 13:41 [PATCH v2 0/8] exec: cleanup search_binary_handler() Oleg Nesterov
2013-08-05 13:41 ` [PATCH v2 1/8] exec: introduce exec_binprm() for "depth == 0" code Oleg Nesterov
2013-08-05 13:41 ` [PATCH v2 2/8] exec: kill "int depth" in search_binary_handler() Oleg Nesterov
2013-08-05 13:41 ` [PATCH v2 3/8] exec: proc_exec_connector() should be called only once Oleg Nesterov
2013-08-05 13:41 ` [PATCH v2 4/8] exec: move allow_write_access/fput to exec_binprm() Oleg Nesterov
2013-08-05 13:41 ` [PATCH v2 5/8] exec: kill ->load_binary != NULL check in search_binary_handler() Oleg Nesterov
2013-08-05 13:41 ` [PATCH v2 6/8] exec: cleanup the CONFIG_MODULES logic Oleg Nesterov
2013-08-05 13:41 ` Oleg Nesterov [this message]
2013-08-05 13:41 ` [PATCH v2 8/8] exec: cleanup the error handling in search_binary_handler() Oleg Nesterov

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=20130805134145.GA15652@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    --cc=zbr@ioremap.net \
    --cc=zml@linux.vnet.ibm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.