| Message ID | 20220127000724.15106-1-ariadne@dereferenced.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] fs/exec: require argv[0] presence in do_execveat_common() | expand |
On Thu, Jan 27, 2022 at 12:07:24AM +0000, Ariadne Conill wrote: > In several other operating systems, it is a hard requirement that the > second argument to execve(2) be the name of a program, thus prohibiting > a scenario where argc < 1. POSIX 2017 also recommends this behaviour, > but it is not an explicit requirement[0]: > > The argument arg0 should point to a filename string that is > associated with the process being started by one of the exec > functions. > > To ensure that execve(2) with argc < 1 is not a useful tool for > shellcode to use, we can validate this in do_execveat_common() and > fail for this scenario, effectively blocking successful exploitation > of CVE-2021-4034 and similar bugs which depend on execve(2) working > with argc < 1. > > We use -EINVAL for this case, mirroring recent changes to FreeBSD and > OpenBSD. -EINVAL is also used by QNX for this, while Solaris uses > -EFAULT. > > In earlier versions of the patch, it was proposed that we create a > fake argv for applications to use when argc < 1, but it was concluded > that it would be better to just fail the execve(2) in these cases, as > launching a process with an empty or NULL argv[0] was likely to just > cause more problems. Let's do it and see what breaks. :) I do see at least tools/testing/selftests/exec/recursion-depth.c will need a fix. And maybe testcases/kernel/syscalls/execveat/execveat.h in LTP. Acked-by: Kees Cook <keescook@chromium.org> > > Interestingly, Michael Kerrisk opened an issue about this in 2008[1], > but there was no consensus to support fixing this issue then. > Hopefully now that CVE-2021-4034 shows practical exploitative use[2] > of this bug in a shellcode, we can reconsider. > > This issue is being tracked in the KSPP issue tracker[3]. > > There are a few[4][5] minor edge cases (primarily in test suites) that > are caught by this, but we plan to work with the projects to fix those > edge cases. > > [0]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html > [1]: https://bugzilla.kernel.org/show_bug.cgi?id=8408 > [2]: https://www.qualys.com/2022/01/25/cve-2021-4034/pwnkit.txt > [3]: https://github.com/KSPP/linux/issues/176 > [4]: https://codesearch.debian.net/search?q=execve%5C+*%5C%28%5B%5E%2C%5D%2B%2C+*NULL&literal=0 > [5]: https://codesearch.debian.net/search?q=execlp%3F%5Cs*%5C%28%5B%5E%2C%5D%2B%2C%5Cs*NULL&literal=0 > > Changes from v2: > - Switch to using -EINVAL as the error code for this. > - Use pr_warn_once() to warn when an execve(2) is rejected due to NULL > argv. > > Changes from v1: > - Rework commit message significantly. > - Make the argv[0] check explicit rather than hijacking the error-check > for count(). > > Reported-by: Michael Kerrisk <mtk.manpages@gmail.com> > To: Andrew Morton <akpm@linux-foundation.org> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Christian Brauner <brauner@kernel.org> > Cc: Rich Felker <dalias@libc.org> > Cc: Eric Biederman <ebiederm@xmission.com> > Cc: Alexander Viro <viro@zeniv.linux.org.uk> > Cc: Kees Cook <keescook@chromium.org> > Cc: linux-fsdevel@vger.kernel.org > Cc: linux-mm@kvack.org > Cc: stable@vger.kernel.org > Signed-off-by: Ariadne Conill <ariadne@dereferenced.org> > --- > fs/exec.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/exec.c b/fs/exec.c > index 79f2c9483302..982730cfe3b8 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1897,6 +1897,10 @@ static int do_execveat_common(int fd, struct filename *filename, > } > > retval = count(argv, MAX_ARG_STRINGS); > + if (retval == 0) { > + pr_warn_once("Attempted to run process '%s' with NULL argv\n", bprm->filename); > + retval = -EINVAL; > + } > if (retval < 0) > goto out_free; > bprm->argc = retval; > -- > 2.34.1 >
Kees Cook <keescook@chromium.org> writes: > On Thu, Jan 27, 2022 at 12:07:24AM +0000, Ariadne Conill wrote: >> In several other operating systems, it is a hard requirement that the >> second argument to execve(2) be the name of a program, thus prohibiting >> a scenario where argc < 1. POSIX 2017 also recommends this behaviour, >> but it is not an explicit requirement[0]: >> >> The argument arg0 should point to a filename string that is >> associated with the process being started by one of the exec >> functions. >> >> To ensure that execve(2) with argc < 1 is not a useful tool for >> shellcode to use, we can validate this in do_execveat_common() and >> fail for this scenario, effectively blocking successful exploitation >> of CVE-2021-4034 and similar bugs which depend on execve(2) working >> with argc < 1. >> >> We use -EINVAL for this case, mirroring recent changes to FreeBSD and >> OpenBSD. -EINVAL is also used by QNX for this, while Solaris uses >> -EFAULT. >> >> In earlier versions of the patch, it was proposed that we create a >> fake argv for applications to use when argc < 1, but it was concluded >> that it would be better to just fail the execve(2) in these cases, as >> launching a process with an empty or NULL argv[0] was likely to just >> cause more problems. > > Let's do it and see what breaks. :) > > I do see at least tools/testing/selftests/exec/recursion-depth.c will > need a fix. And maybe testcases/kernel/syscalls/execveat/execveat.h > in LTP. > > Acked-by: Kees Cook <keescook@chromium.org> Yes since this only appears to be tests that will break. Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> Especially since you are signing up to help fix the tests. >> >> Interestingly, Michael Kerrisk opened an issue about this in 2008[1], >> but there was no consensus to support fixing this issue then. >> Hopefully now that CVE-2021-4034 shows practical exploitative use[2] >> of this bug in a shellcode, we can reconsider. >> >> This issue is being tracked in the KSPP issue tracker[3]. >> >> There are a few[4][5] minor edge cases (primarily in test suites) that >> are caught by this, but we plan to work with the projects to fix those >> edge cases. >> >> [0]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html >> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=8408 >> [2]: https://www.qualys.com/2022/01/25/cve-2021-4034/pwnkit.txt >> [3]: https://github.com/KSPP/linux/issues/176 >> [4]: https://codesearch.debian.net/search?q=execve%5C+*%5C%28%5B%5E%2C%5D%2B%2C+*NULL&literal=0 >> [5]: https://codesearch.debian.net/search?q=execlp%3F%5Cs*%5C%28%5B%5E%2C%5D%2B%2C%5Cs*NULL&literal=0 >> >> Changes from v2: >> - Switch to using -EINVAL as the error code for this. >> - Use pr_warn_once() to warn when an execve(2) is rejected due to NULL >> argv. >> >> Changes from v1: >> - Rework commit message significantly. >> - Make the argv[0] check explicit rather than hijacking the error-check >> for count(). >> >> Reported-by: Michael Kerrisk <mtk.manpages@gmail.com> >> To: Andrew Morton <akpm@linux-foundation.org> >> Cc: Matthew Wilcox <willy@infradead.org> >> Cc: Christian Brauner <brauner@kernel.org> >> Cc: Rich Felker <dalias@libc.org> >> Cc: Eric Biederman <ebiederm@xmission.com> >> Cc: Alexander Viro <viro@zeniv.linux.org.uk> >> Cc: Kees Cook <keescook@chromium.org> >> Cc: linux-fsdevel@vger.kernel.org >> Cc: linux-mm@kvack.org >> Cc: stable@vger.kernel.org >> Signed-off-by: Ariadne Conill <ariadne@dereferenced.org> >> --- >> fs/exec.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/exec.c b/fs/exec.c >> index 79f2c9483302..982730cfe3b8 100644 >> --- a/fs/exec.c >> +++ b/fs/exec.c >> @@ -1897,6 +1897,10 @@ static int do_execveat_common(int fd, struct filename *filename, >> } >> >> retval = count(argv, MAX_ARG_STRINGS); >> + if (retval == 0) { >> + pr_warn_once("Attempted to run process '%s' with NULL argv\n", bprm->filename); >> + retval = -EINVAL; >> + } >> if (retval < 0) >> goto out_free; >> bprm->argc = retval; >> -- >> 2.34.1 >>
On Mon, Jan 31, 2022 at 10:43:52PM +0800, kernel test robot wrote: > > > Greeting, > > FYI, we noticed the following commit (built with gcc-9): > > commit: 80bd5afdd8568e41fc3a75c695bb179e0d9eee4d ("[PATCH v3] fs/exec: require argv[0] presence in do_execveat_common()") > url: https://github.com/0day-ci/linux/commits/Ariadne-Conill/fs-exec-require-argv-0-presence-in-do_execveat_common/20220127-080829 > base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39 > patch link: https://lore.kernel.org/lkml/20220127000724.15106-1-ariadne@dereferenced.org > > in testcase: xfstests > version: xfstests-x86_64-972d710-1_20220127 > with following parameters: > > disk: 4HDD > fs: f2fs > test: generic-group-31 > ucode: 0xe2 > > test-description: xfstests is a regression test suite for xfs and other files ystems. > test-url: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git > > > on test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz with 32G memory > > caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace): > > > > > If you fix the issue, kindly add following tag > Reported-by: kernel test robot <oliver.sang@intel.com> > > > > user :warn : [ 208.077271] run fstests generic/633 at 2022-01-30 04:50:49 > kern :warn : [ 208.529090] Attempted to run process '/dev/fd/5/file1' with NULL argv > user :notice: [ 208.806756] generic/633 [failed, exit status 1]- output mismatch (see /lkp/benchmarks/xfstests/results//generic/633.out.bad) > > user :notice: [ 208.826454] --- tests/generic/633.out 2022-01-27 11:54:16.000000000 +0000 > > user :notice: [ 208.842458] +++ /lkp/benchmarks/xfstests/results//generic/633.out.bad 2022-01-30 04:50:49.769538285 +0000 > > user :notice: [ 208.859622] @@ -1,2 +1,4 @@ > > user :warn : [ 208.860623] run fstests generic/634 at 2022-01-30 04:50:49 > user :notice: [ 208.866037] QA output created by 633 > > user :notice: [ 208.889262] Silence is golden > > user :notice: [ 208.901240] +idmapped-mounts.c: 3608: setid_binaries - Invalid argument - failure: sys_execveat This is from the generic part of the vfs testsuite. It verifies that set*id binaries are executed with the right e{g,u}id. Part of that test calls execveat() as: static char *argv[] = { NULL, }; static char *envp[] = { "EXPECTED_EUID=5000", "EXPECTED_EGID=5000", NULL, }; syscall(__NR_execveat, fd, some_path, argv, envp, 0); I can fix this rather simply in our upstream fstests with: static char *argv[] = { "", }; I guess. But doesn't static char *argv[] = { NULL, }; seem something that should work especially with execveat()? Christian
On Mon, Jan 31, 2022 at 04:08:19PM +0100, Christian Brauner wrote: > On Mon, Jan 31, 2022 at 10:43:52PM +0800, kernel test robot wrote: > I can fix this rather simply in our upstream fstests with: > > static char *argv[] = { > "", > }; > > I guess. > > But doesn't > > static char *argv[] = { > NULL, > }; > > seem something that should work especially with execveat()? The problem is that the exec'ed program sees an argc of 0, which is the problem we're trying to work around in the kernel (instead of leaving it to ld.so to fix for suid programs).
On Mon, Jan 31, 2022 at 03:19:22PM +0000, Matthew Wilcox wrote: > On Mon, Jan 31, 2022 at 04:08:19PM +0100, Christian Brauner wrote: > > On Mon, Jan 31, 2022 at 10:43:52PM +0800, kernel test robot wrote: > > I can fix this rather simply in our upstream fstests with: > > > > static char *argv[] = { > > "", > > }; > > > > I guess. > > > > But doesn't > > > > static char *argv[] = { > > NULL, > > }; > > > > seem something that should work especially with execveat()? > > The problem is that the exec'ed program sees an argc of 0, which is the > problem we're trying to work around in the kernel (instead of leaving > it to ld.so to fix for suid programs). Ok, just seems a bit more intuitive for path-based exec than for fd-based execveat(). What's argv[0] supposed to contain in these cases? 1. execveat(fd, NULL, ..., AT_EMPTY_PATH) 2. execveat(fd, "my-file", ..., ) "" in both 1. and 2.? "" in 1. and "my-file" in 2.?
On Mon, Jan 31, 2022 at 04:37:07PM +0100, Christian Brauner wrote: > On Mon, Jan 31, 2022 at 03:19:22PM +0000, Matthew Wilcox wrote: > > On Mon, Jan 31, 2022 at 04:08:19PM +0100, Christian Brauner wrote: > > > On Mon, Jan 31, 2022 at 10:43:52PM +0800, kernel test robot wrote: > > > I can fix this rather simply in our upstream fstests with: > > > > > > static char *argv[] = { > > > "", > > > }; > > > > > > I guess. > > > > > > But doesn't > > > > > > static char *argv[] = { > > > NULL, > > > }; > > > > > > seem something that should work especially with execveat()? > > > > The problem is that the exec'ed program sees an argc of 0, which is the > > problem we're trying to work around in the kernel (instead of leaving > > it to ld.so to fix for suid programs). > > Ok, just seems a bit more intuitive for path-based exec than for > fd-based execveat(). > > What's argv[0] supposed to contain in these cases? > > 1. execveat(fd, NULL, ..., AT_EMPTY_PATH) > 2. execveat(fd, "my-file", ..., ) > > "" in both 1. and 2.? > "" in 1. and "my-file" in 2.? You didn't specify argv for either of those, so I have no idea. Programs shouldn't be assuming anything about argv[0]; it's purely advisory. Unfortunately, some of them do. And some of them are suid.
On Mon, Jan 31, 2022 at 03:51:21PM +0000, Matthew Wilcox wrote: > On Mon, Jan 31, 2022 at 04:37:07PM +0100, Christian Brauner wrote: > > On Mon, Jan 31, 2022 at 03:19:22PM +0000, Matthew Wilcox wrote: > > > On Mon, Jan 31, 2022 at 04:08:19PM +0100, Christian Brauner wrote: > > > > On Mon, Jan 31, 2022 at 10:43:52PM +0800, kernel test robot wrote: > > > > I can fix this rather simply in our upstream fstests with: > > > > > > > > static char *argv[] = { > > > > "", > > > > }; > > > > > > > > I guess. > > > > > > > > But doesn't > > > > > > > > static char *argv[] = { > > > > NULL, > > > > }; > > > > > > > > seem something that should work especially with execveat()? > > > > > > The problem is that the exec'ed program sees an argc of 0, which is the > > > problem we're trying to work around in the kernel (instead of leaving > > > it to ld.so to fix for suid programs). > > > > Ok, just seems a bit more intuitive for path-based exec than for > > fd-based execveat(). > > > > What's argv[0] supposed to contain in these cases? > > > > 1. execveat(fd, NULL, ..., AT_EMPTY_PATH) > > 2. execveat(fd, "my-file", ..., ) > > > > "" in both 1. and 2.? > > "" in 1. and "my-file" in 2.? > > You didn't specify argv for either of those, so I have no idea. > Programs shouldn't be assuming anything about argv[0]; it's purely > advisory. Unfortunately, some of them do. And some of them are suid. Yes, programs shouldn't assume anything about argv[0]. But a lot of programs are used to setting argv[0] to the name of the executed binary. The exec* manpages examples do this. Just looking at a random selftest, e.g. bpf/prog_tests/test_lsm.c where we find: char *CMD_ARGS[] = {"true", NULL}; execvp(CMD_ARGS[0], CMD_ARGS); I'm just wondering how common this is for execveat() because it is not as clear what the actual name of the binary is in these two examples 1. fd = open("/bin/true", ); char *CMD_ARGS[] = {"", NULL}; execveat(fd, NULL, ..., AT_EMPTY_PATH) 2. fd = open("/bin", ); char *CMD_ARGS[] = {"true", NULL}; execveat(fd, CMD_ARGS[0], CMD_ARGS 0) in other words, the changes that you see CMD_ARGS[0] == NULL for execveat() seem higher than for path-based exec. To counter that we should probably at least update the execveat() manpage with a recommendation what CMD_ARGS[0] should be set to if it isn't allowed to be set to NULL anymore. This is why was asking what argv[0] is supposed to be if the binary doesn't take any arguments.
On Mon, Jan 31, 2022 at 05:14:15PM +0100, Christian Brauner wrote: > On Mon, Jan 31, 2022 at 03:51:21PM +0000, Matthew Wilcox wrote: > > On Mon, Jan 31, 2022 at 04:37:07PM +0100, Christian Brauner wrote: > > > On Mon, Jan 31, 2022 at 03:19:22PM +0000, Matthew Wilcox wrote: > > > > On Mon, Jan 31, 2022 at 04:08:19PM +0100, Christian Brauner wrote: > > > > > On Mon, Jan 31, 2022 at 10:43:52PM +0800, kernel test robot wrote: > > > > > I can fix this rather simply in our upstream fstests with: > > > > > > > > > > static char *argv[] = { > > > > > "", > > > > > }; > > > > > > > > > > I guess. > > > > > > > > > > But doesn't > > > > > > > > > > static char *argv[] = { > > > > > NULL, > > > > > }; > > > > > > > > > > seem something that should work especially with execveat()? > > > > > > > > The problem is that the exec'ed program sees an argc of 0, which is the > > > > problem we're trying to work around in the kernel (instead of leaving > > > > it to ld.so to fix for suid programs). > > > > > > Ok, just seems a bit more intuitive for path-based exec than for > > > fd-based execveat(). > > > > > > What's argv[0] supposed to contain in these cases? > > > > > > 1. execveat(fd, NULL, ..., AT_EMPTY_PATH) > > > 2. execveat(fd, "my-file", ..., ) > > > > > > "" in both 1. and 2.? > > > "" in 1. and "my-file" in 2.? > > > > You didn't specify argv for either of those, so I have no idea. > > Programs shouldn't be assuming anything about argv[0]; it's purely > > advisory. Unfortunately, some of them do. And some of them are suid. > > Yes, programs shouldn't assume anything about argv[0]. But a lot of > programs are used to setting argv[0] to the name of the executed binary. > The exec* manpages examples do this. Just looking at a random selftest, e.g. > > bpf/prog_tests/test_lsm.c > > where we find: > > char *CMD_ARGS[] = {"true", NULL}; > execvp(CMD_ARGS[0], CMD_ARGS); > > I'm just wondering how common this is for execveat() because it is not > as clear what the actual name of the binary is in these two examples > > 1. > fd = open("/bin/true", ); > char *CMD_ARGS[] = {"", NULL}; > execveat(fd, NULL, ..., AT_EMPTY_PATH) > > 2. > fd = open("/bin", ); > char *CMD_ARGS[] = {"true", NULL}; > execveat(fd, CMD_ARGS[0], CMD_ARGS 0) > > in other words, the changes that you see CMD_ARGS[0] == NULL for > execveat() seem higher than for path-based exec. > > To counter that we should probably at least update the execveat() > manpage with a recommendation what CMD_ARGS[0] should be set to if it > isn't allowed to be set to NULL anymore. This is why was asking what > argv[0] is supposed to be if the binary doesn't take any arguments. Sent a fix to our fstests now replacing the argv[0] as NULL with "".
On Mon, 31 Jan 2022 18:13:44 +0100 Christian Brauner <brauner@kernel.org> wrote: > > in other words, the changes that you see CMD_ARGS[0] == NULL for > > execveat() seem higher than for path-based exec. > > > > To counter that we should probably at least update the execveat() > > manpage with a recommendation what CMD_ARGS[0] should be set to if it > > isn't allowed to be set to NULL anymore. This is why was asking what > > argv[0] is supposed to be if the binary doesn't take any arguments. > > Sent a fix to our fstests now replacing the argv[0] as NULL with "". As we hit this check so quickly, I'm thinking that Ariadne's patch "fs/exec: require argv[0] presence in do_execveat_common()" (which added the check) isn't something we'll be able to merge into mainline?
On Mon, Jan 31, 2022 at 01:59:40PM -0800, Andrew Morton wrote: > On Mon, 31 Jan 2022 18:13:44 +0100 Christian Brauner <brauner@kernel.org> wrote: > > > > in other words, the changes that you see CMD_ARGS[0] == NULL for > > > execveat() seem higher than for path-based exec. > > > > > > To counter that we should probably at least update the execveat() > > > manpage with a recommendation what CMD_ARGS[0] should be set to if it > > > isn't allowed to be set to NULL anymore. This is why was asking what > > > argv[0] is supposed to be if the binary doesn't take any arguments. > > > > Sent a fix to our fstests now replacing the argv[0] as NULL with "". > > As we hit this check so quickly, I'm thinking that Ariadne's patch > "fs/exec: require argv[0] presence in do_execveat_common()" (which > added the check) isn't something we'll be able to merge into mainline? I think the next best would be to mutate an NULL argv into { "", NULL }. However, I still think we should do the pr_warn(). Thoughts?
On Mon, Jan 31, 2022 at 01:59:40PM -0800, Andrew Morton wrote: > On Mon, 31 Jan 2022 18:13:44 +0100 Christian Brauner <brauner@kernel.org> wrote: > > > > in other words, the changes that you see CMD_ARGS[0] == NULL for > > > execveat() seem higher than for path-based exec. > > > > > > To counter that we should probably at least update the execveat() > > > manpage with a recommendation what CMD_ARGS[0] should be set to if it > > > isn't allowed to be set to NULL anymore. This is why was asking what > > > argv[0] is supposed to be if the binary doesn't take any arguments. > > > > Sent a fix to our fstests now replacing the argv[0] as NULL with "". > > As we hit this check so quickly, I'm thinking that Ariadne's patch > "fs/exec: require argv[0] presence in do_execveat_common()" (which > added the check) isn't something we'll be able to merge into mainline? Not in the originally envisioned form. But I think Kees patch to make a argv[0] the empty string when it's NULL has a better chance of surviving.
On Mon, Jan 31, 2022 at 02:49:36PM -0800, Kees Cook wrote: > On Mon, Jan 31, 2022 at 01:59:40PM -0800, Andrew Morton wrote: > > On Mon, 31 Jan 2022 18:13:44 +0100 Christian Brauner <brauner@kernel.org> wrote: > > > > > > in other words, the changes that you see CMD_ARGS[0] == NULL for > > > > execveat() seem higher than for path-based exec. > > > > > > > > To counter that we should probably at least update the execveat() > > > > manpage with a recommendation what CMD_ARGS[0] should be set to if it > > > > isn't allowed to be set to NULL anymore. This is why was asking what > > > > argv[0] is supposed to be if the binary doesn't take any arguments. > > > > > > Sent a fix to our fstests now replacing the argv[0] as NULL with "". > > > > As we hit this check so quickly, I'm thinking that Ariadne's patch > > "fs/exec: require argv[0] presence in do_execveat_common()" (which > > added the check) isn't something we'll be able to merge into mainline? > > I think the next best would be to mutate an NULL argv into { "", NULL }. > However, I still think we should do the pr_warn(). > > Thoughts? +1
diff --git a/fs/exec.c b/fs/exec.c index 79f2c9483302..982730cfe3b8 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1897,6 +1897,10 @@ static int do_execveat_common(int fd, struct filename *filename, } retval = count(argv, MAX_ARG_STRINGS); + if (retval == 0) { + pr_warn_once("Attempted to run process '%s' with NULL argv\n", bprm->filename); + retval = -EINVAL; + } if (retval < 0) goto out_free; bprm->argc = retval;
In several other operating systems, it is a hard requirement that the second argument to execve(2) be the name of a program, thus prohibiting a scenario where argc < 1. POSIX 2017 also recommends this behaviour, but it is not an explicit requirement[0]: The argument arg0 should point to a filename string that is associated with the process being started by one of the exec functions. To ensure that execve(2) with argc < 1 is not a useful tool for shellcode to use, we can validate this in do_execveat_common() and fail for this scenario, effectively blocking successful exploitation of CVE-2021-4034 and similar bugs which depend on execve(2) working with argc < 1. We use -EINVAL for this case, mirroring recent changes to FreeBSD and OpenBSD. -EINVAL is also used by QNX for this, while Solaris uses -EFAULT. In earlier versions of the patch, it was proposed that we create a fake argv for applications to use when argc < 1, but it was concluded that it would be better to just fail the execve(2) in these cases, as launching a process with an empty or NULL argv[0] was likely to just cause more problems. Interestingly, Michael Kerrisk opened an issue about this in 2008[1], but there was no consensus to support fixing this issue then. Hopefully now that CVE-2021-4034 shows practical exploitative use[2] of this bug in a shellcode, we can reconsider. This issue is being tracked in the KSPP issue tracker[3]. There are a few[4][5] minor edge cases (primarily in test suites) that are caught by this, but we plan to work with the projects to fix those edge cases. [0]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html [1]: https://bugzilla.kernel.org/show_bug.cgi?id=8408 [2]: https://www.qualys.com/2022/01/25/cve-2021-4034/pwnkit.txt [3]: https://github.com/KSPP/linux/issues/176 [4]: https://codesearch.debian.net/search?q=execve%5C+*%5C%28%5B%5E%2C%5D%2B%2C+*NULL&literal=0 [5]: https://codesearch.debian.net/search?q=execlp%3F%5Cs*%5C%28%5B%5E%2C%5D%2B%2C%5Cs*NULL&literal=0 Changes from v2: - Switch to using -EINVAL as the error code for this. - Use pr_warn_once() to warn when an execve(2) is rejected due to NULL argv. Changes from v1: - Rework commit message significantly. - Make the argv[0] check explicit rather than hijacking the error-check for count(). Reported-by: Michael Kerrisk <mtk.manpages@gmail.com> To: Andrew Morton <akpm@linux-foundation.org> Cc: Matthew Wilcox <willy@infradead.org> Cc: Christian Brauner <brauner@kernel.org> Cc: Rich Felker <dalias@libc.org> Cc: Eric Biederman <ebiederm@xmission.com> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Kees Cook <keescook@chromium.org> Cc: linux-fsdevel@vger.kernel.org Cc: linux-mm@kvack.org Cc: stable@vger.kernel.org Signed-off-by: Ariadne Conill <ariadne@dereferenced.org> --- fs/exec.c | 4 ++++ 1 file changed, 4 insertions(+)