diff --git a/NEWS b/NEWS index 730f2b5ff..924032b3c 100644 --- a/NEWS +++ b/NEWS @@ -93,6 +93,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 449838 sigsegv liburing the 'impossible' happened for io_uring_setup 450025 Powerc: ACC file not implemented as a logical overlay of the VSR registers. +450437 Warn for execve syscall with argv or argv[0] being NULL 450536 Powerpc: valgrind throws 'facility scv unavailable exception' 451626 Syscall param bpf(attr->raw_tracepoint.name) points to unaddressable byte(s) 451827 [ppc64le] VEX temporary storage exhausted with several vbpermq instructions diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c index bc3fa6fe9..44a60bf12 100644 --- a/coregrind/m_syswrap/syswrap-generic.c +++ b/coregrind/m_syswrap/syswrap-generic.c @@ -2933,6 +2933,7 @@ void handle_pre_sys_execve(ThreadId tid, SyscallStatus *status, Addr pathname, Bool setuid_allowed, trace_this_child; const char *str; char str2[30], str3[30]; + Addr arg_2_check = arg_2; switch (execveType) { case EXECVE: @@ -2951,15 +2952,26 @@ void handle_pre_sys_execve(ThreadId tid, SyscallStatus *status, Addr pathname, VG_(strcpy)(str2, str); VG_(strcpy)(str3, str); - if (arg_2 != 0) { - /* At least the terminating NULL must be addressable. */ - if (!ML_(safe_to_deref)((HChar **) (Addr)arg_2, sizeof(HChar *))) { - SET_STATUS_Failure(VKI_EFAULT); - return; + VG_(strcat)(str2, "(argv)"); + VG_(strcat)(str3, "(argv[0])"); + + /* argv[] should not be NULL and valid. */ + PRE_MEM_READ(str2, arg_2_check, sizeof(Addr)); + + /* argv[0] should not be NULL and valid. */ + if (ML_(safe_to_deref)((HChar **) (Addr)arg_2_check, sizeof(HChar *))) { + Addr argv0 = *(Addr*)arg_2_check; + PRE_MEM_RASCIIZ( str3, argv0 ); + /* The rest of argv can be NULL or a valid string pointer. */ + if (VG_(am_is_valid_for_client)(arg_2_check, sizeof(HChar), VKI_PROT_READ)) { + arg_2_check += sizeof(HChar*); + str3[VG_(strlen)(str)] = '\0'; + VG_(strcat)(str3, "(argv[i])"); + ML_(pre_argv_envp)( arg_2_check, tid, str2, str3 ); } - VG_(strcat)(str2, "(argv)"); - VG_(strcat)(str3, "(argv[i])"); - ML_(pre_argv_envp)( arg_2, tid, str2, str3 ); + } else { + SET_STATUS_Failure(VKI_EFAULT); + return; } // Reset helper strings to syscall name. str2[VG_(strlen)(str)] = '\0'; diff --git a/memcheck/tests/arm64-linux/scalar.stderr.exp b/memcheck/tests/arm64-linux/scalar.stderr.exp index 66975efcb..4c81819b6 100644 --- a/memcheck/tests/arm64-linux/scalar.stderr.exp +++ b/memcheck/tests/arm64-linux/scalar.stderr.exp @@ -75,6 +75,11 @@ Syscall param execve(filename) points to unaddressable byte(s) by 0x........: main (scalar.c:91) Address 0x........ is not stack'd, malloc'd or (recently) free'd +Syscall param execve(argv) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:91) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + ----------------------------------------------------- 49: __NR_chdir 1s 1m ----------------------------------------------------- @@ -576,13 +581,13 @@ Syscall param getpriority(who) contains uninitialised byte(s) ----------------------------------------------------- 140: __NR_setpriority 3s 0m ----------------------------------------------------- + +More than 100 errors detected. Subsequent errors +will still be recorded, but in less detail than before. Syscall param setpriority(which) contains uninitialised byte(s) ... by 0x........: main (scalar.c:458) - -More than 100 errors detected. Subsequent errors -will still be recorded, but in less detail than before. Syscall param setpriority(who) contains uninitialised byte(s) ... by 0x........: main (scalar.c:458) diff --git a/memcheck/tests/execve1.c b/memcheck/tests/execve1.c index 83e058a2f..df36f145e 100644 --- a/memcheck/tests/execve1.c +++ b/memcheck/tests/execve1.c @@ -4,7 +4,7 @@ int main(void) { char* null_filename = NULL; char* bad[2] = { (char*)1, NULL }; - char* good[1] = { NULL }; + char* good[2] = { "true", NULL }; execve(null_filename, bad, bad); execve("/bin/true", good, good); diff --git a/memcheck/tests/execve1.stderr.exp b/memcheck/tests/execve1.stderr.exp index 37a91b83a..eebc1e5eb 100644 --- a/memcheck/tests/execve1.stderr.exp +++ b/memcheck/tests/execve1.stderr.exp @@ -3,7 +3,7 @@ Syscall param execve(filename) points to unaddressable byte(s) by 0x........: main (execve1.c:9) Address 0x........ is not stack'd, malloc'd or (recently) free'd -Syscall param execve(argv[i]) points to unaddressable byte(s) +Syscall param execve(argv[0]) points to unaddressable byte(s) ... by 0x........: main (execve1.c:9) Address 0x........ is not stack'd, malloc'd or (recently) free'd diff --git a/memcheck/tests/execve2.stderr.exp b/memcheck/tests/execve2.stderr.exp index cd98593f7..f9d7c3592 100644 --- a/memcheck/tests/execve2.stderr.exp +++ b/memcheck/tests/execve2.stderr.exp @@ -3,3 +3,8 @@ Syscall param execve(filename) points to unaddressable byte(s) by 0x........: main (execve2.c:9) Address 0x........ is not stack'd, malloc'd or (recently) free'd +Syscall param execve(argv) points to unaddressable byte(s) + ... + by 0x........: main (execve2.c:9) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + diff --git a/memcheck/tests/linux/sys-execveat.stderr.exp b/memcheck/tests/linux/sys-execveat.stderr.exp index a58b0fb6a..b49b9be98 100644 --- a/memcheck/tests/linux/sys-execveat.stderr.exp +++ b/memcheck/tests/linux/sys-execveat.stderr.exp @@ -17,3 +17,15 @@ Syscall param execveat(argv) points to uninitialised byte(s) at 0x........: malloc (vg_replace_malloc.c:...) by 0x........: main (sys-execveat.c:41) +Syscall param execveat(argv[0]) points to unaddressable byte(s) + ... + by 0x........: sys_execveat (sys-execveat.c:16) + by 0x........: main (sys-execveat.c:51) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + +Syscall param execveat(argv) points to unaddressable byte(s) + ... + by 0x........: sys_execveat (sys-execveat.c:16) + by 0x........: main (sys-execveat.c:52) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + diff --git a/memcheck/tests/x86-linux/scalar.c b/memcheck/tests/x86-linux/scalar.c index 52f0d4e35..54d0e0443 100644 --- a/memcheck/tests/x86-linux/scalar.c +++ b/memcheck/tests/x86-linux/scalar.c @@ -95,9 +95,9 @@ int main(void) char *argv_envp[] = {(char *) (x0 + 1), NULL}; GO(__NR_execve, "4s 2m"); SY(__NR_execve, x0 + 1, x0 + argv_envp, x0); FAIL; - + char *argv_ok[] = {"frob", NULL}; GO(__NR_execve, "4s 2m"); - SY(__NR_execve, x0 + 1, x0, x0 + argv_envp); FAIL; + SY(__NR_execve, x0 + 1, x0 + argv_ok, x0 + argv_envp); FAIL; // __NR_chdir 12 GO(__NR_chdir, "1s 1m"); diff --git a/memcheck/tests/x86-linux/scalar.stderr.exp b/memcheck/tests/x86-linux/scalar.stderr.exp index 470023f0e..b9202a8c2 100644 --- a/memcheck/tests/x86-linux/scalar.stderr.exp +++ b/memcheck/tests/x86-linux/scalar.stderr.exp @@ -170,6 +170,11 @@ Syscall param execve(filename) points to unaddressable byte(s) by 0x........: main (scalar.c:90) Address 0x........ is not stack'd, malloc'd or (recently) free'd +Syscall param execve(argv) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:90) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + ----------------------------------------------------- 11: __NR_execve 3s 1m ----------------------------------------------------- @@ -190,6 +195,11 @@ Syscall param execve(filename) points to unaddressable byte(s) by 0x........: main (scalar.c:93) Address 0x........ is not stack'd, malloc'd or (recently) free'd +Syscall param execve(argv) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:93) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + ----------------------------------------------------- 11: __NR_execve 4s 2m ----------------------------------------------------- @@ -216,7 +226,7 @@ Syscall param execve(argv) points to uninitialised byte(s) Address 0x........ is on thread 1's stack in frame #1, created by main (scalar.c:29) -Syscall param execve(argv[i]) points to unaddressable byte(s) +Syscall param execve(argv[0]) points to unaddressable byte(s) ... by 0x........: main (scalar.c:97) Address 0x........ is not stack'd, malloc'd or (recently) free'd @@ -564,6 +574,9 @@ Syscall param pipe(filedes) contains uninitialised byte(s) ... by 0x........: main (scalar.c:225) + +More than 100 errors detected. Subsequent errors +will still be recorded, but in less detail than before. Syscall param pipe(filedes) points to unaddressable byte(s) ... by 0x........: main (scalar.c:225) @@ -576,9 +589,6 @@ Syscall param times(buf) contains uninitialised byte(s) ... by 0x........: main (scalar.c:229) - -More than 100 errors detected. Subsequent errors -will still be recorded, but in less detail than before. Syscall param times(buf) points to unaddressable byte(s) ... by 0x........: main (scalar.c:229) diff --git a/none/tests/execve.c b/none/tests/execve.c index 950842da2..a1af72fd9 100644 --- a/none/tests/execve.c +++ b/none/tests/execve.c @@ -7,20 +7,42 @@ int main(int argc, char **argv) if (argc == 1) { // This tests the case where argv and envp are NULL, which is easy to - // get wrong because it's an unusual case. + // get wrong because it's an unusual case. It is also bad and only + // "worked" by accident with the linux kernel. + + char *const argv_exe[] = {"true", NULL}; + char *const v_null[] = { NULL }; + char *const v_minus_one[] = { (char *const) -1, NULL }; #if defined(VGO_solaris) - // Solaris requires non-NULL argv parameter - char *const argv_exe[] = {"true", NULL}; - if (execve("/bin/true", argv_exe, NULL) < 0) + const char *exe = "/bin/true"; #elif defined(VGO_darwin) - if (execve("/usr/bin/true", NULL, NULL) < 0) + const char *exe = "/usr/bin/true"; #elif defined(VGO_freebsd) - char *const argv_exe[] = {"true", NULL}; - if (execve("/usr/bin/true", argv_exe, NULL) < 0) + const char *exe = "/usr/bin/true"; #else - if (execve("/bin/true", NULL, NULL) < 0) + const char *exe = "/bin/true"; #endif + + /* Try some bad argv and envp arguments, make sure the executable + doesn't actually exists, so execve doesn't accidentally succeeds. */ + if (execve("/%/", NULL, NULL) >= 0) + printf ("WHAT?"); + if (execve("/%/", (void *)-1, NULL) >= 0) + printf ("WHAT?"); + if (execve("/%/", v_null, NULL) >= 0) + printf ("WHAT?"); + if (execve("/%/", v_null, v_null) >= 0) + printf ("WHAT?"); + if (execve("/%/", v_minus_one, NULL) >= 0) + printf ("WHAT?"); + if (execve("/%/", v_minus_one, v_null) >= 0) + printf ("WHAT?"); + if (execve("/%/", v_minus_one, v_minus_one) >= 0) + printf ("WHAT?"); + + /* Finally a correct execve. */ + if (execve(exe, argv_exe, NULL) < 0) { perror("execve"); exit(1);