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
next prev 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.