Menu
▾
▴
[Valgrind-developers] [valgrind/VALGRIND_3_23_BRANCH] Don't leave fds created with --log-file, --xml-file or --log-socket open
|
From: Mark W. <ma...@so...> - 2024-06-23 20:07:58
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=14cefe7c645a3148165f4b2fa6095d9446e378c7 commit 14cefe7c645a3148165f4b2fa6095d9446e378c7 Author: Mark Wielaard <ma...@kl...> Date: Sun Jun 16 21:23:08 2024 +0200 Don't leave fds created with --log-file, --xml-file or --log-socket open prepare_sink_fd and prepare_sink_socket will create a new file descriptor for the output sink. finalize_sink_fd then copies the fd to the safe range, so it doesn't conflict with any application fds. If we created the original fd ourselves, it was a VgLogTo_File or VgLogTo_Socket, not VgLogTo_Fd, finalize_sink_fd should close it. Also close socket when connecting fails in VG_(connect_via_socket). Add a testcase for --log-file and --xml-file which prints output to /dev/stderr https://bugs.kde.org/show_bug.cgi?id=202770 https://bugs.kde.org/show_bug.cgi?id=311655 https://bugs.kde.org/show_bug.cgi?id=488379 Co-authored-by: Alexandra Hájková <aha...@re... (cherry picked from commit fbd7596f8342f0b0fbbe088d960da839a8bdb839) Diff: --- NEWS | 3 +++ coregrind/m_libcfile.c | 1 + coregrind/m_libcprint.c | 6 +++++ none/tests/Makefile.am | 5 +++- none/tests/filter_xml | 25 ++++++++++++++++++++ none/tests/log-track-fds.stderr.exp | 0 none/tests/log-track-fds.vgtest | 4 ++++ none/tests/xml-track-fds.stderr.exp | 47 +++++++++++++++++++++++++++++++++++++ none/tests/xml-track-fds.vgtest | 5 ++++ 9 files changed, 95 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index fbe0f012ef..10b5ae3195 100644 --- a/NEWS +++ b/NEWS @@ -5,11 +5,14 @@ Branch 3.23 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 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 487439 SIGILL in JDK11, JDK17 +488379 --track-fds=yes errors that cannot be suppressed with --xml-file= n-i-bz aarch64 frinta and frinta vector instructions To see details of a given bug, visit diff --git a/coregrind/m_libcfile.c b/coregrind/m_libcfile.c index 6098bc5813..9635b80a68 100644 --- a/coregrind/m_libcfile.c +++ b/coregrind/m_libcfile.c @@ -1333,6 +1333,7 @@ Int VG_(connect_via_socket)( const HChar* str ) res = my_connect(sd, &servAddr, sizeof(servAddr)); if (res < 0) { /* connection failed */ + VG_(close)(sd); return -2; } diff --git a/coregrind/m_libcprint.c b/coregrind/m_libcprint.c index c802f81403..593889da9d 100644 --- a/coregrind/m_libcprint.c +++ b/coregrind/m_libcprint.c @@ -425,6 +425,12 @@ static void finalize_sink_fd(OutputSink *sink, Int new_fd, Bool is_xml) } else { VG_(fcntl)(safe_fd, VKI_F_SETFD, VKI_FD_CLOEXEC); sink->fd = safe_fd; + /* If we created the new_fd (VgLogTo_File or VgLogTo_Socket), then we + don't need the original file descriptor open anymore. We only need + to keep it open if it was an existing fd given by the user (or + stderr). */ + if (sink->type != VgLogTo_Fd) + VG_(close)(new_fd); } } diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am index 185993f204..532cc7632a 100644 --- a/none/tests/Makefile.am +++ b/none/tests/Makefile.am @@ -86,6 +86,7 @@ dist_noinst_SCRIPTS = \ filter_none_discards \ filter_stderr \ filter_timestamp \ + filter_xml \ allexec_prepare_prereq noinst_HEADERS = fdleak.h @@ -229,7 +230,9 @@ EXTRA_DIST = \ sigprocmask.stderr.exp sigprocmask.vgtest \ socket_close.stderr.exp socket_close.vgtest \ file_dclose.stderr.exp file_dclose.vgtest \ - double_close_range.stderr.exp double_close_range.vgtest + double_close_range.stderr.exp double_close_range.vgtest \ + log-track-fds.stderr.exp log-track-fds.vgtest \ + xml-track-fds.stderr.exp xml-track-fds.vgtest check_PROGRAMS = \ diff --git a/none/tests/filter_xml b/none/tests/filter_xml new file mode 100755 index 0000000000..d1ef570a05 --- /dev/null +++ b/none/tests/filter_xml @@ -0,0 +1,25 @@ +#! /bin/sh + +dir=`dirname $0` + +# FreeBSD adds this one extra line +# but after filter_xml_frames it will just be <path>...<\/path> +# which matches other lines, so get rid of it while we can +# uniquely match it +sed "/<path>internet<\/path>/d" | + +$dir/../../tests/filter_xml_frames | +perl -p -e "s/<time>.*<\/time>/<time>...<\/time>/s" | +perl -p -e "s/<what>.*<\/what>/<what>...<\/what>/s" | +perl -p -e "s/<path>.*<\/path>/<path>...<\/path>/s" | +perl -p -e "s/<line>Copyright.*<\/line>/<line>Copyright...<\/line>/s" | +perl -p -e "s/<line>Using Valgrind.*<\/line>/<line>Using Valgrind...<\/line>/s" | +sed "s/<ppid>[0-9]*<\/ppid>/<ppid>...<\/ppid>/" | +sed "s/<tid>[0-9]*<\/tid>/<tid>...<\/tid>/" | +sed "s/<pid>[0-9]*<\/pid>/<pid>...<\/pid>/" | +sed "s/<obj>.*<\/obj>/<obj>...<\/obj>/" | +sed "s/<exe>.*<\/exe>/<exe>...<\/exe>/" | +sed "s/<dir>.*<\/dir>/<dir>...<\/dir>/" | +sed "s/<ppid>[0-9]*<\/ppid>/<ppid>...<\/ppid>/" | +sed "s/<unique>0x[0-9a-fA-F]*<\/unique>/<unique>0x........<\/unique>/" | +sed "s/<ip>0x[0-9a-fA-F]*<\/ip>/<ip>0x........<\/ip>/" diff --git a/none/tests/log-track-fds.stderr.exp b/none/tests/log-track-fds.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/log-track-fds.vgtest b/none/tests/log-track-fds.vgtest new file mode 100644 index 0000000000..dfebb5bf3e --- /dev/null +++ b/none/tests/log-track-fds.vgtest @@ -0,0 +1,4 @@ +# Simple test to make sure track-fds doesn't error on (internal) log-file +# See https://bugs.kde.org/show_bug.cgi?id=311655 +prog: ../../tests/true +vgopts: -q --track-fds=yes --log-file=/dev/stderr diff --git a/none/tests/xml-track-fds.stderr.exp b/none/tests/xml-track-fds.stderr.exp new file mode 100644 index 0000000000..b06da9d722 --- /dev/null +++ b/none/tests/xml-track-fds.stderr.exp @@ -0,0 +1,47 @@ +<?xml version="1.0"?> + +<valgrindoutput> + +<protocolversion>5</protocolversion> +<protocoltool>none</protocoltool> + +<preamble> + <line>Nulgrind, the minimal Valgrind tool</line> + <line>Copyright...</line> + <line>Using Valgrind...</line> + <line>Command: ./../../tests/true</line> +</preamble> + +<pid>...</pid> +<ppid>...</ppid> +<tool>none</tool> + +<args> + <vargv> + <exe>...</exe> + <arg>--command-line-only=yes</arg> + <arg>--memcheck:leak-check=no</arg> + <arg>--tool=none</arg> + <arg>--track-fds=yes</arg> + <arg>--xml=yes</arg> + <arg>--xml-file=/dev/stderr</arg> + </vargv> + <argv> + <exe>...</exe> + </argv> +</args> + +<status> + <state>RUNNING</state> + <time>...</time> +</status> + + +<status> + <state>FINISHED</state> + <time>...</time> +</status> + + +</valgrindoutput> + diff --git a/none/tests/xml-track-fds.vgtest b/none/tests/xml-track-fds.vgtest new file mode 100644 index 0000000000..50f1a55a82 --- /dev/null +++ b/none/tests/xml-track-fds.vgtest @@ -0,0 +1,5 @@ +# Simple test to make sure track-fds doesn't error on (internal) xml-file +# See https://bugs.kde.org/show_bug.cgi?id=488379 +prog: ../../tests/true +vgopts: --track-fds=yes --xml=yes --xml-file=/dev/stderr +stderr_filter: filter_xml |
