From: Mark Wielaard Date: Sun, 16 Jun 2024 19:23:08 +0000 (+0200) Subject: Don't leave fds created with --log-file, --xml-file or --log-socket open X-Git-Tag: VALGRIND_3_24_0~109 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fbd7596f8342f0b0fbbe088d960da839a8bdb839;p=thirdparty%2Fvalgrind.git 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á IR: unhandled instruction bytes: 0x66 0xF 0x3A 0x2 +311655 --log-file=FILE leads to apparent fd leak 377966 arm64 unhandled instruction dc zva392146 aarch64: unhandled instruction 0xD5380001 (MRS rT, midr_el1) 392146 aarch64: unhandled instruction 0xD5380001 (MRS rT, midr_el1) @@ -42,6 +44,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 487439 SIGILL in JDK11, JDK17 487993 Alignment error when using Eigen with Valgrind and -m32 488026 Use of `sizeof` instead of `strlen +488379 --track-fds=yes errors that cannot be suppressed with --xml-file= 488441 Add tests for --track-fds=yes --xml=yes and fd suppression tests To see details of a given bug, visit diff --git a/coregrind/m_libcfile.c b/coregrind/m_libcfile.c index 6098bc581..9635b80a6 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 c802f8140..593889da9 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 7caa8ca32..553a9c865 100644 --- a/none/tests/Makefile.am +++ b/none/tests/Makefile.am @@ -251,7 +251,9 @@ EXTRA_DIST = \ file_dclose_xml.stderr.exp file_dclose_xml.vgtest \ file_dclose_xml.stderr.exp-nomain \ file_dclose.supp file_dclose_sup.stderr.exp file_dclose_sup.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/log-track-fds.stderr.exp b/none/tests/log-track-fds.stderr.exp new file mode 100644 index 000000000..e69de29bb diff --git a/none/tests/log-track-fds.vgtest b/none/tests/log-track-fds.vgtest new file mode 100644 index 000000000..dfebb5bf3 --- /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 000000000..b06da9d72 --- /dev/null +++ b/none/tests/xml-track-fds.stderr.exp @@ -0,0 +1,47 @@ + + + + +5 +none + + + Nulgrind, the minimal Valgrind tool + Copyright... + Using Valgrind... + Command: ./../../tests/true + + +... +... +none + + + + ... + --command-line-only=yes + --memcheck:leak-check=no + --tool=none + --track-fds=yes + --xml=yes + --xml-file=/dev/stderr + + + ... + + + + + RUNNING + + + + + + FINISHED + + + + + + diff --git a/none/tests/xml-track-fds.vgtest b/none/tests/xml-track-fds.vgtest new file mode 100644 index 000000000..50f1a55a8 --- /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