| Summary: | arch_prctl should return EINVAL on unknown option | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Mark Wielaard <mark> |
| Component: | general | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | drankinatty, subhranilmukherjee190, vandah3, zubeyir99 |
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| URL: | https://bugzilla.redhat.com/show_bug.cgi?id=1608824 | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
commit 21a01b13e259b9a43f10f0046b2b3f409c11ea75 Author: Mark Wielaard <mark@klomp.org> Date: Mon Jul 30 12:20:16 2018 +0200 Bug 396887 - arch_prctl should return EINVAL on unknown option. Currently arch_prctl calls VG_(core_panic) when it sees an unknown arch_prctl option which kills the process. glibc uses arch_prctl with an (as yet) unknown option to see if the kernel supports CET. This breaks any application running under valgrind on x86_64 with: valgrind: the 'impossible' happened: Unsupported arch_prctl option Thread 1: status = VgTs_Runnable (lwpid 19934) ==19934== at 0x121A15: get_cet_status (cpu-features.c:28) ==19934== by 0x121A15: init_cpu_features (cpu-features.c:474) ==19934== by 0x121A15: dl_platform_init (dl-machine.h:228) ==19934== by 0x121A15: _dl_sysdep_start (dl-sysdep.c:231) ==19934== by 0x10A1D7: _dl_start_final (rtld.c:413) ==19934== by 0x10A1D7: _dl_start (rtld.c:520) We already handle all known options. It would be better to do as the kernel does and just return failure with EINVAL instead. *** Bug 397286 has been marked as a duplicate of this bug. *** *** Bug 397521 has been marked as a duplicate of this bug. *** *** Bug 397393 has been marked as a duplicate of this bug. *** *** Bug 431829 has been marked as a duplicate of this bug. *** |
Currently arch_prctl calls VG_(core_panic) when it sees an unknown arch_prctl which kills the process. glibc uses arch_prctl with a (as yet) unknown option to see if the kernel supports CET. This breaks any application with: valgrind: the 'impossible' happened: Unsupported arch_prctl option host stacktrace: ==19934== at 0x5803B102: ??? (in /usr/lib64/valgrind/memcheck-amd64-linux) ==19934== by 0x5803B214: ??? (in /usr/lib64/valgrind/memcheck-amd64-linux) ==19934== by 0x5803B459: ??? (in /usr/lib64/valgrind/memcheck-amd64-linux) ==19934== by 0x5803B480: ??? (in /usr/lib64/valgrind/memcheck-amd64-linux) ==19934== by 0x580CFD08: ??? (in /usr/lib64/valgrind/memcheck-amd64-linux) ==19934== by 0x58096FFA: ??? (in /usr/lib64/valgrind/memcheck-amd64-linux) ==19934== by 0x58093A72: ??? (in /usr/lib64/valgrind/memcheck-amd64-linux) ==19934== by 0x58095206: ??? (in /usr/lib64/valgrind/memcheck-amd64-linux) ==19934== by 0x580A4ACA: ??? (in /usr/lib64/valgrind/memcheck-amd64-linux) sched status: running_tid=1 Thread 1: status = VgTs_Runnable (lwpid 19934) ==19934== at 0x121A15: get_cet_status (cpu-features.c:28) ==19934== by 0x121A15: init_cpu_features (cpu-features.c:474) ==19934== by 0x121A15: dl_platform_init (dl-machine.h:228) ==19934== by 0x121A15: _dl_sysdep_start (dl-sysdep.c:231) ==19934== by 0x10A1D7: _dl_start_final (rtld.c:413) ==19934== by 0x10A1D7: _dl_start (rtld.c:520) ==19934== by 0x109117: ??? (in /builddir/build/BUILD/glibc-2.27.9000-645-gcfba5dbb10/build-x86_64-redhat-linux/elf/ld.so) ==19934== by 0x3: ??? ==19934== by 0x1FFF0009E6: ??? ==19934== by 0x1FFF0009F0: ??? ==19934== by 0x1FFF0009FF: ??? ==19934== by 0x1FFF000A10: ??? We already handle all known options. It would be better to do as the kernel does and just return failure with EINVAL instead. Proposed patch: diff --git a/coregrind/m_syswrap/syswrap-amd64-linux.c b/coregrind/m_syswrap/syswrap-amd64-linux.c index 0f2ad8c51..407af7f76 100644 --- a/coregrind/m_syswrap/syswrap-amd64-linux.c +++ b/coregrind/m_syswrap/syswrap-amd64-linux.c @@ -249,6 +249,7 @@ PRE(sys_rt_sigreturn) PRE(sys_arch_prctl) { ThreadState* tst; + Bool known_option = True; PRINT( "arch_prctl ( %ld, %lx )", SARG1, ARG2 ); vg_assert(VG_(is_valid_tid)(tid)); @@ -283,13 +284,16 @@ PRE(sys_arch_prctl) POST_MEM_WRITE(ARG2, sizeof(unsigned long)); } else { - VG_(core_panic)("Unsupported arch_prctl option"); + known_option = False; } /* Note; the Status writeback to guest state that happens after this wrapper returns does not change guest_FS_CONST or guest_GS_CONST; hence that direct assignment to the guest state is safe here. */ - SET_STATUS_Success( 0 ); + if (known_option) + SET_STATUS_Success( 0 ); + else + SET_STATUS_Failure( VKI_EINVAL ); } // Parts of this are amd64-specific, but the *PEEK* cases are generic.