exec: kill ->load_binary != NULL check in search_binary_handler()
authorOleg Nesterov <oleg@redhat.com>
Wed, 11 Sep 2013 21:24:42 +0000 (14:24 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Wed, 11 Sep 2013 22:59:05 +0000 (15:59 -0700)
search_binary_handler() checks ->load_binary != NULL for no reason, this
method should be always defined.  Turn this check into WARN_ON() and move
it into __register_binfmt().

Also, kill the function pointer.  The current code looks confusing, as if
->load_binary can go away after read_unlock(&binfmt_lock).  But we rely on
module_get(fmt->module), this fmt can't be changed or unregistered,
otherwise this code is buggy anyway.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Kees Cook <keescook@chromium.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Evgeniy Polyakov <zbr@ioremap.net>
Cc: Zach Levis <zml@linux.vnet.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
fs/exec.c

index a4cfd1d725e099a7d1a3bb86a6f0121b757b75c7..7b92fbfa63aa2c6b7852afa25d4dc43146bc9d48 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -74,6 +74,8 @@ static DEFINE_RWLOCK(binfmt_lock);
 void __register_binfmt(struct linux_binfmt * fmt, int insert)
 {
        BUG_ON(!fmt);
+       if (WARN_ON(!fmt->load_binary))
+               return;
        write_lock(&binfmt_lock);
        insert ? list_add(&fmt->lh, &formats) :
                 list_add_tail(&fmt->lh, &formats);
@@ -1389,14 +1391,11 @@ int search_binary_handler(struct linux_binprm *bprm)
        for (try=0; try<2; try++) {
                read_lock(&binfmt_lock);
                list_for_each_entry(fmt, &formats, lh) {
-                       int (*fn)(struct linux_binprm *) = fmt->load_binary;
-                       if (!fn)
-                               continue;
                        if (!try_module_get(fmt->module))
                                continue;
                        read_unlock(&binfmt_lock);
                        bprm->recursion_depth++;
-                       retval = fn(bprm);
+                       retval = fmt->load_binary(bprm);
                        bprm->recursion_depth--;
                        if (retval >= 0) {
                                put_binfmt(fmt);