From: Aaron Merey Date: Wed, 26 Jan 2022 01:24:18 +0000 (-0500) Subject: Bug 445011: SIGCHLD is sent when valgrind uses debuginfod-find X-Git-Tag: VALGRIND_3_19_0~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2ad93350446a49a5b0093548b63d43195d99d4ae;p=thirdparty%2Fvalgrind.git Bug 445011: SIGCHLD is sent when valgrind uses debuginfod-find Valgrind fork+execs debuginfod-find in order to perform debuginfod queries. Any SIGCHLD debuginfod-find sends upon termination can mistakenly be delivered to the client running under valgrind. To prevent this, record in a hash table the PID of each process valgrind forks for internal use. Do not send SIGCHLD to the client if it is from a PID in this hash table. https://bugs.kde.org/show_bug.cgi?id=445011 --- diff --git a/NEWS b/NEWS index 924032b3c1..d3bb86d0f6 100644 --- a/NEWS +++ b/NEWS @@ -72,6 +72,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 445032 valgrind/memcheck crash with SIGSEGV when SIGVTALRM timer used and libthr.so associated 445300 [PATCH] Fix building tests with Musl +445011 SIGCHLD is sent when valgrind uses debuginfod-find 445354 arm64 backend: incorrect code emitted for doubleword CAS 445415 arm64 front end: alignment checks missing for atomic instructions 445504 Using C++ condition_variable results in bogus "mutex is locked simultaneously by two threads" warning diff --git a/coregrind/m_libcproc.c b/coregrind/m_libcproc.c index 7425c9c887..b94cabcf1f 100644 --- a/coregrind/m_libcproc.c +++ b/coregrind/m_libcproc.c @@ -38,6 +38,7 @@ #include "pub_core_libcsignal.h" #include "pub_core_seqmatch.h" #include "pub_core_mallocfree.h" +#include "pub_core_signals.h" #include "pub_core_syscall.h" #include "pub_core_xarray.h" #include "pub_core_clientstate.h" @@ -888,14 +889,68 @@ Int VG_(ptrace) ( Int request, Int pid, void *addr, void *data ) Fork ------------------------------------------------------------------ */ +/* Record PID of a child process in order to avoid sending any SIGCHLD from + it to the client. If PID is 0 then this is the child process and it + should synch with the parent to ensure it can't send any SIGCHLD before + the parent has registered its PID. + + FDS should be initialized with VG_(pipe). This function closes both + file descriptors. */ +static void register_sigchld_ignore ( Int pid, Int fds[2]) +{ + Int child_wait = 1; + ht_ignore_node *n; + + if (fds[0] < 0 || fds[1] < 0) + return; + + if (pid == 0) { + /* Before proceeding, ensure parent has recorded child PID in map + of SIGCHLD to ignore */ + while (child_wait == 1) + { + if (VG_(read)(fds[0], &child_wait, sizeof(Int)) <= 0) { + VG_(message)(Vg_DebugMsg, + "warning: Unable to record PID of internal process (read)\n"); + child_wait = 0; + } + } + + VG_(close)(fds[0]); + return; + } + + n = VG_(malloc)("ht.ignore.node", sizeof(ht_ignore_node)); + n->key = pid; + if (ht_sigchld_ignore == NULL) + ht_sigchld_ignore = VG_(HT_construct)("ht.sigchld.ignore"); + VG_(HT_add_node)(ht_sigchld_ignore, n); + + child_wait = 0; + if (VG_(write)(fds[1], &child_wait, sizeof(Int)) <= 0) + VG_(message)(Vg_DebugMsg, + "warning: Unable to record PID of internal process (write)\n"); + + VG_(close)(fds[1]); +} + Int VG_(fork) ( void ) { + Int fds[2]; + + if (VG_(pipe)(fds) != 0) { + VG_(message)(Vg_DebugMsg, + "warning: Unable to record PID of internal process (pipe)\n"); + fds[0] = fds[1] = -1; + } + # if defined(VGP_arm64_linux) || defined(VGP_nanomips_linux) SysRes res; res = VG_(do_syscall5)(__NR_clone, VKI_SIGCHLD, (UWord)NULL, (UWord)NULL, (UWord)NULL, (UWord)NULL); if (sr_isError(res)) return -1; + register_sigchld_ignore(sr_Res(res), fds); return sr_Res(res); # elif defined(VGO_linux) || defined(VGO_freebsd) @@ -903,6 +958,7 @@ Int VG_(fork) ( void ) res = VG_(do_syscall0)(__NR_fork); if (sr_isError(res)) return -1; + register_sigchld_ignore(sr_Res(res), fds); return sr_Res(res); # elif defined(VGO_darwin) @@ -912,8 +968,10 @@ Int VG_(fork) ( void ) return -1; /* on success: wLO = child pid; wHI = 1 for child, 0 for parent */ if (sr_ResHI(res) != 0) { + register_sigchld_ignore(0, fds); return 0; /* this is child: return 0 instead of child pid */ } + register_sigchld_ignore(sr_Res(res), fds); return sr_Res(res); # elif defined(VGO_solaris) @@ -930,8 +988,10 @@ Int VG_(fork) ( void ) child, val2 = 0 in the parent process, 1 in the child process. */ if (sr_ResHI(res) != 0) { + register_sigchld_ignore(0, fds); return 0; } + register_sigchld_ignore(sr_Res(res), fds); return sr_Res(res); # else diff --git a/coregrind/m_signals.c b/coregrind/m_signals.c index 9210db8e5e..bfddbe392a 100644 --- a/coregrind/m_signals.c +++ b/coregrind/m_signals.c @@ -207,6 +207,7 @@ #include "pub_core_aspacemgr.h" #include "pub_core_errormgr.h" #include "pub_core_gdbserver.h" +#include "pub_core_hashtable.h" #include "pub_core_libcbase.h" #include "pub_core_libcassert.h" #include "pub_core_libcprint.h" @@ -247,6 +248,9 @@ typedef struct SigQueue { vki_siginfo_t sigs[N_QUEUED_SIGNALS]; } SigQueue; +/* Hash table of PIDs from which SIGCHLD is ignored. */ +VgHashTable *ht_sigchld_ignore = NULL; + /* ------ Macros for pulling stuff out of ucontexts ------ */ /* Q: what does VG_UCONTEXT_SYSCALL_SYSRES do? A: let's suppose the @@ -2058,6 +2062,28 @@ static void deliver_signal ( ThreadId tid, const vki_siginfo_t *info, void *handler_fn; ThreadState *tst = VG_(get_ThreadState)(tid); +#if defined(VGO_linux) + /* If this signal is SIGCHLD and it came from a process which valgrind + created for some internal use, then it should not be delivered to + the client. */ + if (sigNo == VKI_SIGCHLD && ht_sigchld_ignore != NULL) { + Int pid = info->_sifields._sigchld._pid; + ht_ignore_node *n = VG_(HT_lookup)(ht_sigchld_ignore, pid); + + if (n != NULL) { + /* If the child has terminated, remove its PID from the + ignore list. */ + if (info->si_code == VKI_CLD_EXITED + || info->si_code == VKI_CLD_KILLED + || info->si_code == VKI_CLD_DUMPED) { + VG_(HT_remove)(ht_sigchld_ignore, pid); + VG_(free)(n); + } + return; + } + } +#endif + if (VG_(clo_trace_signals)) VG_(dmsg)("delivering signal %d (%s):%d to thread %u\n", sigNo, VG_(signame)(sigNo), info->si_code, tid ); diff --git a/coregrind/pub_core_signals.h b/coregrind/pub_core_signals.h index ae8555ba80..1c86ee5f63 100644 --- a/coregrind/pub_core_signals.h +++ b/coregrind/pub_core_signals.h @@ -35,6 +35,7 @@ #include "pub_tool_signals.h" // I want to get rid of this header... #include "pub_core_vki.h" // vki_sigset_t et al. +#include "pub_tool_hashtable.h" /* Highest signal the kernel will let us use */ extern Int VG_(max_signal); @@ -85,6 +86,15 @@ extern Bool VG_(extend_stack)(ThreadId tid, Addr addr); before using that signal to kill the process. */ extern void VG_(set_default_handler)(Int sig); +/* Hash table of PIDs from which SIGCHLD is ignored. */ +extern VgHashTable *ht_sigchld_ignore; + +/* Hash table node where each key represents a PID. */ +typedef struct _ht_ignore_node { + struct _ht_ignore_node *next; + UWord key; +} ht_ignore_node; + #endif // __PUB_CORE_SIGNALS_H /*--------------------------------------------------------------------*/ diff --git a/include/vki/vki-darwin.h b/include/vki/vki-darwin.h index dbae64bc49..5a01c3cbd9 100644 --- a/include/vki/vki-darwin.h +++ b/include/vki/vki-darwin.h @@ -525,6 +525,12 @@ typedef #define VKI_BUS_ADRERR BUS_ADRERR #define VKI_BUS_OBJERR BUS_OBJERR #define VKI_TRAP_BRKPT TRAP_BRKPT +#define VKI_CLD_EXITED CLD_EXITED +#define VKI_CLD_KILLED CLD_KILLED +#define VKI_CLD_DUMPED CLD_DUMPED +#define VKI_CLD_TRAPPED CLD_TRAPPED +#define VKI_CLD_STOPPED CLD_STOPPED +#define VKI_CLD_CONTINUED CLD_CONTINUED /* JRS: not 100% sure, but I think these two are correct */ #define VKI_SA_ONESHOT SA_RESETHAND diff --git a/include/vki/vki-freebsd.h b/include/vki/vki-freebsd.h index d467f017e5..f50598e093 100644 --- a/include/vki/vki-freebsd.h +++ b/include/vki/vki-freebsd.h @@ -609,6 +609,16 @@ typedef struct vki_siginfo { #define VKI_TRAP_DTRACE 3 /* DTrace induced trap. */ #define VKI_TRAP_CAP 4 /* Capabilities protective trap. */ +/* + * SIGCHLD si_codes + */ +#define VKI_CLD_EXITED 1 /* child has exited */ +#define VKI_CLD_KILLED 2 /* child was killed */ +#define VKI_CLD_DUMPED 3 /* child terminated abnormally */ +#define VKI_CLD_TRAPPED 4 /* traced child has trapped */ +#define VKI_CLD_STOPPED 5 /* child has stopped */ +#define VKI_CLD_CONTINUED 6 /* stopped child has continued */ + #if 0 /* freebsd-6 */ typedef struct vki_sigevent { int sigev_notify; diff --git a/include/vki/vki-linux.h b/include/vki/vki-linux.h index e6cef7dfff..905c3ba36e 100644 --- a/include/vki/vki-linux.h +++ b/include/vki/vki-linux.h @@ -576,6 +576,16 @@ typedef struct vki_siginfo { #define VKI_TRAP_BRKPT (__VKI_SI_FAULT|1) /* process breakpoint */ #define VKI_TRAP_TRACE (__VKI_SI_FAULT|2) /* process trace trap */ +/* + * SIGCHLD si_codes + */ +#define VKI_CLD_EXITED (__VKI_SI_FAULT|1) /* child has exited */ +#define VKI_CLD_KILLED (__VKI_SI_FAULT|2) /* child was killed */ +#define VKI_CLD_DUMPED (__VKI_SI_FAULT|3) /* child terminated abnormally */ +#define VKI_CLD_TRAPPED (__VKI_SI_FAULT|4) /* traced child has trapped */ +#define VKI_CLD_STOPPED (__VKI_SI_FAULT|5) /* child has stopped */ +#define VKI_CLD_CONTINUED (__VKI_SI_FAULT|6) /* stopped child has continued */ + /* * This works because the alignment is ok on all current architectures * but we leave open this being overridden in the future