exec: move allow_write_access/fput to exec_binprm()

When search_binary_handler() succeeds it does allow_write_access() and fput(), then it clears bprm->file to ensure the caller will not do the same. We can simply move this code to exec_binprm() which is called only once. In fact we could move this to free_bprm() and remove the same code in do_execve_common's error path. Signed-off-by: Oleg Nesterov <[email protected]> Acked-by: Kees Cook <[email protected]> Cc: Al Viro <[email protected]> Cc: Evgeniy Polyakov <[email protected]> Cc: Zach Levis <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
iegor · Sep 11, 2013 · 52f1428 · 52f1428
1 parent 9beb266
commit 52f1428
Showing 1 changed file with 6 additions and 4 deletions.
diff --git a/fs/exec.c b/fs/exec.c
@@ -1400,10 +1400,6 @@ int search_binary_handler(struct linux_binprm *bprm)
 			bprm->recursion_depth--;
 			if (retval >= 0) {
 				put_binfmt(fmt);
-				allow_write_access(bprm->file);
-				if (bprm->file)
-					fput(bprm->file);
-				bprm->file = NULL;
 				return retval;
 			}
 			read_lock(&binfmt_lock);
@@ -1455,6 +1451,12 @@ static int exec_binprm(struct linux_binprm *bprm)
 		ptrace_event(PTRACE_EVENT_EXEC, old_vpid);
 		current->did_exec = 1;
 		proc_exec_connector(current);
+
+		if (bprm->file) {
+			allow_write_access(bprm->file);
+			fput(bprm->file);
+			bprm->file = NULL; /* to catch use-after-free */
+		}
 	}
 
 	return ret;