From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
Zach Levis <zml@linux.vnet.ibm.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
Evgeniy Polyakov <zbr@ioremap.net>,
Kees Cook <keescook@chromium.org>,
linux-kernel@vger.kernel.org
Subject: [PATCH 4/5] exec: don't retry if request_module() fails
Date: Fri, 2 Aug 2013 21:27:41 +0200 [thread overview]
Message-ID: <20130802192741.GA9579@redhat.com> (raw)
In-Reply-To: <20130802192713.GA9543@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 addition "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>
---
fs/exec.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 48344a2..d9fd32c 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-02 19:33 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-02 19:27 [PATCH 0/5] exec: more cleanups Oleg Nesterov
2013-08-02 19:27 ` [PATCH 1/5] exec: move allow_write_access/fput to exec_binprm() Oleg Nesterov
2013-08-03 19:27 ` Kees Cook
2013-08-04 14:48 ` Oleg Nesterov
2013-08-02 19:27 ` [PATCH 2/5] exec: kill ->load_binary != NULL check in search_binary_handler() Oleg Nesterov
2013-08-02 19:27 ` [PATCH 3/5] exec: cleanup the CONFIG_MODULES logic Oleg Nesterov
2013-08-02 19:27 ` Oleg Nesterov [this message]
2013-08-02 19:27 ` [PATCH 5/5] exec: cleanup the error handling in search_binary_handler() Oleg Nesterov
2013-08-03 19:28 ` [PATCH 0/5] exec: more cleanups Kees Cook
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=20130802192741.GA9579@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.