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+*... [5]: https://codesearch.debian.net/search?q=execlp%3F%5Cs*%5C%28%5B%5E%2C%5D%2B%2...
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;
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.
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.
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
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... 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
user :notice: [ 208.918907] +idmapped-mounts.c: 13953: run_test - Success - failure: setid binaries on regular mounts
user :notice: [ 208.935261] ...
user :notice: [ 208.949376] (Run 'diff -u /lkp/benchmarks/xfstests/tests/generic/633.out /lkp/benchmarks/xfstests/results//generic/633.out.bad' to see the entire diff)
To reproduce:
git clone https://github.com/intel/lkp-tests.git cd lkp-tests sudo bin/lkp install job.yaml # job file is attached in this email bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run sudo bin/lkp run generated-yaml-file
# if come across any failure that blocks the test, # please remove ~/.lkp and /lkp dir to run from a clean state.
--- 0DAY/LKP+ Test Infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/lkp@lists.01.org Intel Corporation
Thanks, Oliver Sang
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... 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?
- execveat(fd, NULL, ..., AT_EMPTY_PATH)
- 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?
- execveat(fd, NULL, ..., AT_EMPTY_PATH)
- 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?
- execveat(fd, NULL, ..., AT_EMPTY_PATH)
- 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
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 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
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.
linux-stable-mirror@lists.linaro.org
-
Andrew Morton -
Ariadne Conill -
Christian Brauner -
Eric W. Biederman -
Kees Cook -
kernel test robot -
Matthew Wilcox