From 0ced5269236c527d2e288b773e3684b6b1673ec2 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Mon, 17 Jun 2024 00:27:12 +0200 Subject: [PATCH 14/15] Close both internal pipe fds after VG_(fork) in parent and child An VG_fork() creates a pipe between parent and child to syncronize the two processes. The parent wants to register the child pid before the child can run. This is done in register_sigchld_ignore. Make sure both the parent and the child close both the read and write file descriptors so none leak. https://bugs.kde.org/show_bug.cgi?id=479661 (cherry picked from commit 1263471efdf8405cb0f1a767c6af73bf2eaf7160) --- NEWS | 1 + coregrind/m_libcproc.c | 7 +++++++ none/tests/Makefile.am | 3 +++ none/tests/track-fds-exec-children.c | 13 +++++++++++++ none/tests/track-fds-exec-children.stderr.exp | 0 none/tests/track-fds-exec-children.vgtest | 3 +++ 6 files changed, 27 insertions(+) create mode 100644 none/tests/track-fds-exec-children.c create mode 100644 none/tests/track-fds-exec-children.stderr.exp create mode 100644 none/tests/track-fds-exec-children.vgtest diff --git a/NEWS b/NEWS index 10b5ae3195ca..94789a04ba9e 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,7 @@ The following bugs have been fixed or resolved on this branch. 202770 open fd at exit --log-socket=127.0.0.1:1500 with --track-fds=yes 311655 --log-file=FILE leads to apparent fd leak 453044 gbserver_tests failures in aarch64 +479661 Valgrind leaks file descriptors 486180 [MIPS] 'VexGuestArchState' has no member named 'guest_IP_AT_SYSCALL' 486293 memccpy false positives 486569 linux inotify_init syscall wrapper missing POST entry in syscall_table diff --git a/coregrind/m_libcproc.c b/coregrind/m_libcproc.c index 11dabe768f59..8422e9d1187a 100644 --- a/coregrind/m_libcproc.c +++ b/coregrind/m_libcproc.c @@ -905,6 +905,8 @@ static void register_sigchld_ignore ( Int pid, Int fds[2]) return; if (pid == 0) { + /* We are the child, close writing fd that we don't use. */ + VG_(close)(fds[1]); /* Before proceeding, ensure parent has recorded child PID in map of SIGCHLD to ignore */ while (child_wait == 1) @@ -916,6 +918,7 @@ static void register_sigchld_ignore ( Int pid, Int fds[2]) } } + /* Now close reading fd. */ VG_(close)(fds[0]); return; } @@ -926,11 +929,15 @@ static void register_sigchld_ignore ( Int pid, Int fds[2]) ht_sigchld_ignore = VG_(HT_construct)("ht.sigchld.ignore"); VG_(HT_add_node)(ht_sigchld_ignore, n); + /* We are the parent process, close read fd that we don't use. */ + VG_(close)(fds[0]); + 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"); + /* Now close writing fd. */ VG_(close)(fds[1]); } diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am index 532cc7632a8f..1751ad3fa2e5 100644 --- a/none/tests/Makefile.am +++ b/none/tests/Makefile.am @@ -223,6 +223,7 @@ EXTRA_DIST = \ threadederrno.vgtest \ timestamp.stderr.exp timestamp.vgtest \ tls.vgtest tls.stderr.exp tls.stdout.exp \ + track-fds-exec-children.vgtest track-fds-exec-children.stderr.exp \ unit_debuglog.stderr.exp unit_debuglog.vgtest \ vgprintf.stderr.exp vgprintf.vgtest \ vgprintf_nvalgrind.stderr.exp vgprintf_nvalgrind.vgtest \ @@ -276,6 +277,7 @@ check_PROGRAMS = \ tls \ tls.so \ tls2.so \ + track-fds-exec-children \ unit_debuglog \ valgrind_cpp_test \ vgprintf \ @@ -414,6 +416,7 @@ if VGCONF_OS_IS_DARWIN else tls2_so_LDFLAGS = -shared endif +track_fds_exec_children_SOURCES = track-fds-exec-children.c vgprintf_nvalgrind_SOURCES = vgprintf.c vgprintf_nvalgrind_CFLAGS = ${AM_CFLAGS} -DNVALGRIND diff --git a/none/tests/track-fds-exec-children.c b/none/tests/track-fds-exec-children.c new file mode 100644 index 000000000000..7209ee73d5b7 --- /dev/null +++ b/none/tests/track-fds-exec-children.c @@ -0,0 +1,13 @@ +#include +#include + +int main() +{ + pid_t pid = fork (); + if (pid == 0) + execlp("true", "true", NULL); + + // Wait till true succeeds + wait (NULL); + return 0; +} diff --git a/none/tests/track-fds-exec-children.stderr.exp b/none/tests/track-fds-exec-children.stderr.exp new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/none/tests/track-fds-exec-children.vgtest b/none/tests/track-fds-exec-children.vgtest new file mode 100644 index 000000000000..aa926a6290ed --- /dev/null +++ b/none/tests/track-fds-exec-children.vgtest @@ -0,0 +1,3 @@ +env: DEBUGINFOD_URLS=file:/dev/null +prog: track-fds-exec-children +vgopts: -q --track-fds=yes --trace-children=yes -- 2.45.2