From 8b72ca9004ed35104deb80b07990da5503bc5252 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Wed, 13 May 2020 22:25:20 -0500 Subject: exec: Move the call of prepare_binprm into search_binary_handler The code in prepare_binary_handler needs to be run every time search_binary_handler is called so move the call into search_binary_handler itself to make the code simpler and easier to understand. Link: https://lkml.kernel.org/r/87d070zrvx.fsf_-_@x220.int.ebiederm.org Acked-by: Linus Torvalds Reviewed-by: Kees Cook Reviewed-by: James Morris Signed-off-by: "Eric W. Biederman" --- arch/alpha/kernel/binfmt_loader.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'arch/alpha') diff --git a/arch/alpha/kernel/binfmt_loader.c b/arch/alpha/kernel/binfmt_loader.c index a8d0d6e06526..d712ba51d15a 100644 --- a/arch/alpha/kernel/binfmt_loader.c +++ b/arch/alpha/kernel/binfmt_loader.c @@ -35,9 +35,6 @@ static int load_binary(struct linux_binprm *bprm) bprm->file = file; bprm->loader = loader; - retval = prepare_binprm(bprm); - if (retval < 0) - return retval; return search_binary_handler(bprm); } -- cgit v1.2.3 From bc2bf338d54b7aadaed49bb45b9e10d4592b2a46 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Mon, 18 May 2020 18:43:20 -0500 Subject: 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 Reviewed-by: Kees Cook Signed-off-by: "Eric W. Biederman" --- arch/alpha/kernel/binfmt_loader.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'arch/alpha') diff --git a/arch/alpha/kernel/binfmt_loader.c b/arch/alpha/kernel/binfmt_loader.c index d712ba51d15a..e4be7a543ecf 100644 --- a/arch/alpha/kernel/binfmt_loader.c +++ b/arch/alpha/kernel/binfmt_loader.c @@ -19,10 +19,6 @@ static int load_binary(struct linux_binprm *bprm) if (bprm->loader) return -ENOEXEC; - allow_write_access(bprm->file); - fput(bprm->file); - bprm->file = NULL; - loader = bprm->vma->vm_end - sizeof(void *); file = open_exec("/sbin/loader"); @@ -33,9 +29,9 @@ static int load_binary(struct linux_binprm *bprm) /* Remember if the application is TASO. */ bprm->taso = eh->ah.entry < 0x100000000UL; - bprm->file = file; + bprm->interpreter = file; bprm->loader = loader; - return search_binary_handler(bprm); + return 0; } static struct linux_binfmt loader_format = { -- cgit v1.2.3