exec: Add comments on check_unsafe_exec() fs counting

Add some comments about what the fs counting is doing in
check_unsafe_exec() and how it relates to the call graph.
Specifically, we can't force an unshare of the fs because
of at least Chrome:
https://lore.kernel.org/lkml/86CE201B-5632-4BB7-BCF6-7CB2C2895409@chromium.org/

Cc: Eric Biederman <ebiederm@xmission.com>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Link: https://lore.kernel.org/r/20221018071537.never.662-kees@kernel.org

diff --git a/fs/exec.c b/fs/exec.c
index 34e6a2e532680c4c4ad094a14629225b087ecb71..b3e87bd50962353055eacc1b5a2570982d01663b 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1573,6 +1573,12 @@ static void check_unsafe_exec(struct linux_binprm *bprm)
        if (task_no_new_privs(current))
                bprm->unsafe |= LSM_UNSAFE_NO_NEW_PRIVS;
 
+       /*
+        * If another task is sharing our fs, we cannot safely
+        * suid exec because the differently privileged task
+        * will be able to manipulate the current directory, etc.
+        * It would be nice to force an unshare instead...
+        */
        t = p;
        n_fs = 1;
        spin_lock(&p->fs->lock);
@@ -1753,6 +1759,7 @@ static int search_binary_handler(struct linux_binprm *bprm)
        return retval;
 }
 
+/* binfmt handlers will call back into begin_new_exec() on success. */
 static int exec_binprm(struct linux_binprm *bprm)
 {
        pid_t old_pid, old_vpid;
@@ -1811,6 +1818,11 @@ static int bprm_execve(struct linux_binprm *bprm,
        if (retval)
                return retval;
 
+       /*
+        * Check for unsafe execution states before exec_binprm(), which
+        * will call back into begin_new_exec(), into bprm_creds_from_file(),
+        * where setuid-ness is evaluated.
+        */
        check_unsafe_exec(bprm);
        current->in_execve = 1;