tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

exec: don't WARN for racy path_noexec check

Both i_mode and noexec checks wrapped in WARN_ON stem from an artifact
of the previous implementation. They used to legitimately check for the
condition, but that got moved up in two commits:
633fb6ac3980 ("exec: move S_ISREG() check earlier")
0fd338b2d2cd ("exec: move path_noexec() check earlier")

Instead of being removed said checks are WARN_ON'ed instead, which
has some debug value.

However, the spurious path_noexec check is racy, resulting in
unwarranted warnings should someone race with setting the noexec flag.

One can note there is more to perm-checking whether execve is allowed
and none of the conditions are guaranteed to still hold after they were
tested for.

Additionally this does not validate whether the code path did any perm
checking to begin with -- it will pass if the inode happens to be
regular.

Keep the redundant path_noexec() check even though it's mindless
nonsense checking for guarantee that isn't given so drop the WARN.

Reword the commentary and do small tidy ups while here.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
Link: https://lore.kernel.org/r/20240805131721.765484-1-mjguzik@gmail.com
[brauner: keep redundant path_noexec() check]
Signed-off-by: Christian Brauner <brauner@kernel.org>

authored by

Mateusz Guzik and committed by
Christian Brauner 0d196e75 46460c1d

+12 -19
+12 -19
fs/exec.c
··· 145 145 goto out; 146 146 147 147 /* 148 - * may_open() has already checked for this, so it should be 149 - * impossible to trip now. But we need to be extra cautious 150 - * and check again at the very end too. 148 + * Check do_open_execat() for an explanation. 151 149 */ 152 150 error = -EACCES; 153 - if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) || 154 - path_noexec(&file->f_path))) 151 + if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode)) || 152 + path_noexec(&file->f_path)) 155 153 goto exit; 156 154 157 155 error = -ENOEXEC; ··· 952 954 static struct file *do_open_execat(int fd, struct filename *name, int flags) 953 955 { 954 956 struct file *file; 955 - int err; 956 957 struct open_flags open_exec_flags = { 957 958 .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC, 958 959 .acc_mode = MAY_EXEC, ··· 968 971 969 972 file = do_filp_open(fd, name, &open_exec_flags); 970 973 if (IS_ERR(file)) 971 - goto out; 974 + return file; 972 975 973 976 /* 974 - * may_open() has already checked for this, so it should be 975 - * impossible to trip now. But we need to be extra cautious 976 - * and check again at the very end too. 977 + * In the past the regular type check was here. It moved to may_open() in 978 + * 633fb6ac3980 ("exec: move S_ISREG() check earlier"). Since then it is 979 + * an invariant that all non-regular files error out before we get here. 977 980 */ 978 - err = -EACCES; 979 - if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) || 980 - path_noexec(&file->f_path))) 981 - goto exit; 981 + if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode)) || 982 + path_noexec(&file->f_path)) { 983 + fput(file); 984 + return ERR_PTR(-EACCES); 985 + } 982 986 983 - out: 984 987 return file; 985 - 986 - exit: 987 - fput(file); 988 - return ERR_PTR(err); 989 988 } 990 989 991 990 /**