diff options
author | Eric W. Biederman <ebiederm@xmission.com> | 2020-05-18 18:43:20 -0500 |
---|---|---|
committer | Eric W. Biederman <ebiederm@xmission.com> | 2020-05-21 10:16:57 -0500 |
commit | bc2bf338d54b7aadaed49bb45b9e10d4592b2a46 (patch) | |
tree | 796c65aeab06fd03839d5f7324a5ea70743b0b2d /fs/binfmt_misc.c | |
parent | b8a61c9e7b4a0fec493d191429e9653d66a79ccc (diff) | |
download | lwn-bc2bf338d54b7aadaed49bb45b9e10d4592b2a46.tar.gz lwn-bc2bf338d54b7aadaed49bb45b9e10d4592b2a46.zip |
exec: Remove recursion from search_binary_handler
Recursion in kernel code is generally a bad idea as it can overflow
the kernel stack. Recursion in exec also hides that the code is
looping and that the loop changes bprm->file.
Instead of recursing in search_binary_handler have the methods that
would recurse set bprm->interpreter and return 0. Modify exec_binprm
to loop when bprm->interpreter is set. Consolidate all of the
reassignments of bprm->file in that loop to make it clear what is
going on.
The structure of the new loop in exec_binprm is that all errors return
immediately, while successful completion (ret == 0 &&
!bprm->interpreter) just breaks out of the loop and runs what
exec_bprm has always run upon successful completion.
Fail if the an interpreter is being call after execfd has been set.
The code has never properly handled an interpreter being called with
execfd being set and with reassignments of bprm->file and the
assignment of bprm->executable in generic code it has finally become
possible to test and fail when if this problematic condition happens.
With the reassignments of bprm->file and the assignment of
bprm->executable moved into the generic code add a test to see if
bprm->executable is being reassigned.
In search_binary_handler remove the test for !bprm->file. With all
reassignments of bprm->file moved to exec_binprm bprm->file can never
be NULL in search_binary_handler.
Link: https://lkml.kernel.org/r/87sgfwyd84.fsf_-_@x220.int.ebiederm.org
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Diffstat (limited to 'fs/binfmt_misc.c')
-rw-r--r-- | fs/binfmt_misc.c | 18 |
1 files changed, 3 insertions, 15 deletions
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index ad2866f28f0c..53968ea07b57 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -159,18 +159,9 @@ static int load_misc_binary(struct linux_binprm *bprm) goto ret; } - if (fmt->flags & MISC_FMT_OPEN_BINARY) { - /* Pass the open binary to the interpreter */ + if (fmt->flags & MISC_FMT_OPEN_BINARY) bprm->have_execfd = 1; - bprm->executable = bprm->file; - allow_write_access(bprm->file); - bprm->file = NULL; - } else { - allow_write_access(bprm->file); - fput(bprm->file); - bprm->file = NULL; - } /* make argv[1] be the path to the binary */ retval = copy_strings_kernel(1, &bprm->interp, bprm); if (retval < 0) @@ -199,14 +190,11 @@ static int load_misc_binary(struct linux_binprm *bprm) if (IS_ERR(interp_file)) goto ret; - bprm->file = interp_file; + bprm->interpreter = interp_file; if (fmt->flags & MISC_FMT_CREDENTIALS) bprm->preserve_creds = 1; - retval = search_binary_handler(bprm); - if (retval < 0) - goto ret; - + retval = 0; ret: dput(fmt->dentry); return retval; |