exec: Add do_close_execat() helper

Consolidate the calls to allow_write_access()/fput() into a single
place, since we repeat this code pattern. Add comments around the
callers for the details on it.

Link: https://lore.kernel.org/r/202209161637.9EDAF6B18@keescook
Signed-off-by: Kees Cook <keescook@chromium.org>

diff --git a/fs/exec.c b/fs/exec.c
index ba7d0548ac57f0f3a7f66a71e7d6665730fa0877..2037cc6360364523af63e8eb6669585faf73c685 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -904,6 +904,10 @@ EXPORT_SYMBOL(transfer_args_to_stack);
 
 #endif /* CONFIG_MMU */
 
+/*
+ * On success, caller must call do_close_execat() on the returned
+ * struct file to close it.
+ */
 static struct file *do_open_execat(int fd, struct filename *name, int flags)
 {
        struct file *file;
@@ -948,6 +952,17 @@ exit:
        return ERR_PTR(err);
 }
 
+/**
+ * open_exec - Open a path name for execution
+ *
+ * @name: path name to open with the intent of executing it.
+ *
+ * Returns ERR_PTR on failure or allocated struct file on success.
+ *
+ * As this is a wrapper for the internal do_open_execat(), callers
+ * must call allow_write_access() before fput() on release. Also see
+ * do_close_execat().
+ */
 struct file *open_exec(const char *name)
 {
        struct filename *filename = getname_kernel(name);
@@ -1484,6 +1499,15 @@ static int prepare_bprm_creds(struct linux_binprm *bprm)
        return -ENOMEM;
 }
 
+/* Matches do_open_execat() */
+static void do_close_execat(struct file *file)
+{
+       if (!file)
+               return;
+       allow_write_access(file);
+       fput(file);
+}
+
 static void free_bprm(struct linux_binprm *bprm)
 {
        if (bprm->mm) {
@@ -1495,10 +1519,7 @@ static void free_bprm(struct linux_binprm *bprm)
                mutex_unlock(&current->signal->cred_guard_mutex);
                abort_creds(bprm->cred);
        }
-       if (bprm->file) {
-               allow_write_access(bprm->file);
-               fput(bprm->file);
-       }
+       do_close_execat(bprm->file);
        if (bprm->executable)
                fput(bprm->executable);
        /* If a binfmt changed the interp, free it. */
@@ -1520,8 +1541,7 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl
 
        bprm = kzalloc(sizeof(*bprm), GFP_KERNEL);
        if (!bprm) {
-               allow_write_access(file);
-               fput(file);
+               do_close_execat(file);
                return ERR_PTR(-ENOMEM);
        }