From: Alexandra Hájková Date: Tue, 12 Aug 2025 16:17:54 +0000 (-0400) Subject: Filter Valgrind FDs from getdents syscalls X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e8e4066c3a0160f03d9dfffaa360b65eb79745d1;p=thirdparty%2Fvalgrind.git Filter Valgrind FDs from getdents syscalls This change prevents client programs from seeing Valgrind's internal file descriptors when scanning /proc/self/fd or /proc//fd. This patch modifies the getdents and getdents64 syscall wrappers to selectively filter out Valgrind's internal file descriptors only when listing /proc/*/fd directories for the current process. Add none/tests/getdents_filter.vgtest test that tests that the Valgrind's file descriptors are hidden from the client program and verifies both /proc/self/fd filtering and that regular directory listings remain unfiltered. https://bugs.kde.org/show_bug.cgi?id=331311 --- diff --git a/.gitignore b/.gitignore index e70c9cc42..de362ecc2 100644 --- a/.gitignore +++ b/.gitignore @@ -1701,6 +1701,7 @@ /none/tests/vgprintf /none/tests/vgprintf_nvalgrind /none/tests/yield +/none/tests/getdents_filter # /none/tests/amd64/ /none/tests/amd64/*.dSYM diff --git a/NEWS b/NEWS index ecd58cc50..d7f0cb3ba 100644 --- a/NEWS +++ b/NEWS @@ -67,6 +67,7 @@ bugzilla (https://bugs.kde.org/enter_bug.cgi?product=valgrind) rather than mailing the developers (or mailing lists) directly -- bugs that are not entered into bugzilla tend to get forgotten about or ignored. +331311 Valgrind shows open files in /proc/self/fd that don't work for the process 309554 Wrap syscall remap_file_pages (216) 338803 Handling of dwz debug alt files or cross-CU is broken 369030 Wrap linux syscall: 171 (setdomainname) diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c index ce4c11c26..33b58f5a5 100644 --- a/coregrind/m_syswrap/syswrap-generic.c +++ b/coregrind/m_syswrap/syswrap-generic.c @@ -3994,11 +3994,181 @@ PRE(sys_getdents) PRE_MEM_WRITE( "getdents(dirp)", ARG2, ARG3 ); } +/* Check if the given file descriptor points to a /proc/PID/fd or /proc/PID/fdinfo directory. + Returns True if it's a directory we should filter Valgrind FDs from. */ +static Bool is_proc_fd_directory(Int fd) +{ + const HChar* path; + if (!VG_(resolve_filename)(fd, &path)) + return False; + + HChar ppfd[32]; /* large enough for /proc//fd and /proc//fdinfo */ + HChar ppfdinfo[32]; + VG_(sprintf)(ppfd, "/proc/%d/fd", VG_(getpid)()); + VG_(sprintf)(ppfdinfo, "/proc/%d/fdinfo", VG_(getpid)()); + + if (VG_(strcmp)(path, "/proc/self/fd") == 0 + || VG_(strcmp)(path, "/proc/self/fdinfo") == 0 + || VG_(strcmp)(path, ppfd) == 0 + || VG_(strcmp)(path, ppfdinfo) == 0) + return True; + + return False; +} + + +/* Helper function to decide if an FD entry should be kept */ +static Bool should_keep_fd_entry(const HChar *name) +{ + if (VG_(strcmp)(name, ".") == 0 || VG_(strcmp)(name, "..") == 0) + return True; + + Bool is_numeric = True; + for (Int i = 0; name[i] != '\0'; i++) { + if (name[i] < '0' || name[i] > '9') { + is_numeric = False; + if (VG_(clo_verbosity) > 1) + VG_(dmsg)("Warning: Non-numeric entry '%s' found in /proc/*/fd directory\n", name); + break; + } + } + + if (is_numeric && name[0] != '\0') { + Int fd_num = VG_(strtoll10)(name, NULL); + /* Hide FDs beyond soft limit or Valgrind's output FDs */ + if (fd_num >= VG_(fd_soft_limit) || + fd_num == VG_(log_output_sink).fd || + fd_num == VG_(xml_output_sink).fd) + return False; + } + + return True; +} + +/* Filter and compact dirent entries */ +static SizeT filter_dirent_entries(struct vki_dirent *dirp, SizeT orig_size) +{ + struct vki_dirent *dp = dirp; + struct vki_dirent *write_pos = dirp; + SizeT bytes_processed = 0; + SizeT new_size = 0; + + while (bytes_processed < orig_size) { + if (should_keep_fd_entry(dp->d_name)) { + if (write_pos != dp) + VG_(memmove)(write_pos, dp, dp->d_reclen); + new_size += dp->d_reclen; + write_pos = (struct vki_dirent *)((HChar *)write_pos + dp->d_reclen); + } + + bytes_processed += dp->d_reclen; + dp = (struct vki_dirent *)((HChar *)dp + dp->d_reclen); + } + + return new_size; +} + +/* Filter and compact dirent64 entries */ +static SizeT filter_dirent64_entries(struct vki_dirent64 *dirp, SizeT orig_size) +{ + struct vki_dirent64 *dp = dirp; + struct vki_dirent64 *write_pos = dirp; + SizeT bytes_processed = 0; + SizeT new_size = 0; + + while (bytes_processed < orig_size) { + if (should_keep_fd_entry(dp->d_name)) { + if (write_pos != dp) + VG_(memmove)(write_pos, dp, dp->d_reclen); + new_size += dp->d_reclen; + write_pos = (struct vki_dirent64 *)((HChar *)write_pos + dp->d_reclen); + } + + bytes_processed += dp->d_reclen; + dp = (struct vki_dirent64 *)((HChar *)dp + dp->d_reclen); + } + + return new_size; +} + +/* Filter out Valgrind's internal file descriptors from getdents results with refill capability. + When entries are filtered out, attempts to read more entries to avoid empty results. + Returns filtered size on success, or -1 if retry syscall failed. */ +static SizeT filter_valgrind_fds_from_getdents_with_refill(Int fd, struct vki_dirent *dirp, SizeT orig_size, SizeT buf_size, /*MOD*/SyscallStatus* status) +{ + SizeT new_size = filter_dirent_entries(dirp, orig_size); + + /* If we filtered out everything, try to read more entries. + Since new_size == 0, the buffer is completely unused. */ + while (new_size == 0) { + SysRes retry = VG_(do_syscall3)(__NR_getdents, fd, (UWord)dirp, buf_size); + if (sr_isError(retry)) { + /* Set the syscall status and return error indicator */ + SET_STATUS_from_SysRes(retry); + return -1; + } + if (sr_Res(retry) == 0) { + /* Real EOF - stop trying */ + break; + } + + /* Filter the new batch */ + SizeT retry_size = sr_Res(retry); + new_size = filter_dirent_entries(dirp, retry_size); + } + + return new_size; +} + +/* Filter out Valgrind's internal file descriptors from getdents64 results with refill capability. + Same logic as getdents version but for 64-bit dirent structures. + Returns filtered size on success, or -1 if retry syscall failed. */ +static SizeT filter_valgrind_fds_from_getdents64_with_refill(Int fd, struct vki_dirent64 *dirp, SizeT orig_size, SizeT buf_size, /*MOD*/SyscallStatus* status) +{ + SizeT new_size = filter_dirent64_entries(dirp, orig_size); + + /* If we filtered out everything, try to read more entries. + Since new_size == 0, the buffer is completely unused. */ + while (new_size == 0) { + SysRes retry = VG_(do_syscall3)(__NR_getdents64, fd, (UWord)dirp, buf_size); + if (sr_isError(retry)) { + /* Set the syscall status and return error indicator */ + SET_STATUS_from_SysRes(retry); + return -1; + } + if (sr_Res(retry) == 0) { + /* Real EOF - stop trying */ + break; + } + + /* Filter the new batch */ + SizeT retry_size = sr_Res(retry); + new_size = filter_dirent64_entries(dirp, retry_size); + } + + return new_size; +} + POST(sys_getdents) { vg_assert(SUCCESS); - if (RES > 0) - POST_MEM_WRITE( ARG2, RES ); + if (RES > 0) { + SizeT result_size = RES; + + /* Only filter Valgrind FDs when listing /proc/PID/fd or /proc/PID/fdinfo directories */ + if (is_proc_fd_directory(ARG1)) { + SizeT filtered_size = filter_valgrind_fds_from_getdents_with_refill(ARG1, (struct vki_dirent *)ARG2, RES, ARG3, status); + if ((SizeT)filtered_size == (SizeT)-1) { + /* Error already set by filter function */ + return; + } + result_size = filtered_size; + if (result_size != RES) + SET_STATUS_Success(result_size); + } + + POST_MEM_WRITE( ARG2, result_size ); + } } PRE(sys_getdents64) @@ -4015,8 +4185,23 @@ PRE(sys_getdents64) POST(sys_getdents64) { vg_assert(SUCCESS); - if (RES > 0) - POST_MEM_WRITE( ARG2, RES ); + if (RES > 0) { + SizeT result_size = RES; + + /* Only filter Valgrind FDs when listing /proc/PID/fd or /proc/PID/fdinfo directories */ + if (is_proc_fd_directory(ARG1)) { + SizeT filtered_size = filter_valgrind_fds_from_getdents64_with_refill(ARG1, (struct vki_dirent64 *)ARG2, RES, ARG3, status); + if ((SizeT)filtered_size == (SizeT)-1) { + /* Error already set by filter function */ + return; + } + result_size = filtered_size; + if (result_size != RES) + SET_STATUS_Success(result_size); + } + + POST_MEM_WRITE( ARG2, result_size ); + } } PRE(sys_getgroups) diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am index 716ce000d..ccdd85e53 100644 --- a/none/tests/Makefile.am +++ b/none/tests/Makefile.am @@ -172,6 +172,7 @@ EXTRA_DIST = \ floored.stderr.exp floored.stdout.exp floored.vgtest \ fork.stderr.exp fork.stdout.exp fork.vgtest \ fucomip.stderr.exp fucomip.vgtest \ + getdents_filter.stderr.exp getdents_filter.stdout.exp getdents_filter.vgtest \ gxx304.stderr.exp gxx304.vgtest \ ifunc.stderr.exp ifunc.stdout.exp ifunc.vgtest \ ioctl_moans.stderr.exp ioctl_moans.vgtest \ @@ -291,6 +292,7 @@ check_PROGRAMS = \ fdleak_fcntl fdleak_ipv4 fdleak_open fdleak_pipe \ fdleak_socketpair \ floored fork fucomip \ + getdents_filter \ ioctl_moans \ libvex_test \ libvexmultiarch_test \ diff --git a/none/tests/getdents_filter.c b/none/tests/getdents_filter.c new file mode 100644 index 000000000..d508cdde0 --- /dev/null +++ b/none/tests/getdents_filter.c @@ -0,0 +1,159 @@ +/* Regression test for bug #331311: Valgrind file descriptors visible in /proc/self/fd and /proc/self/fdinfo */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +struct linux_dirent { + unsigned long d_ino; + off_t d_off; + unsigned short d_reclen; + char d_name[]; +}; + +/* Small buffer size to test retry logic when all entries get filtered */ +#define SMALL_BUF_SIZE 64 + +/* Helper function to scan a /proc directory and print numeric FD entries with a prefix. + Returns 0 on success, -1 if directory unavailable */ +static int scan_proc_fd_directory(const char *dir_path, const char *prefix) +{ + DIR *d = opendir(dir_path); + if (d == NULL) { + printf("%s not available\n", dir_path); + return -1; + } + + struct dirent *entry; + while ((entry = readdir(d)) != NULL) { + if (entry->d_name[0] == '.') + continue; + + char *endptr; + long fd = strtol(entry->d_name, &endptr, 10); + if (*endptr == '\0' && fd >= 0) { + if (prefix) { + printf("%s:%ld\n", prefix, fd); + } else { + printf("%ld\n", fd); + } + } + } + closedir(d); + return 0; +} + +/* + * Test retry logic with raw getdents syscall and small buffer. + * + * This test validates the filtering refill mechanism in syswrap-generic.c. + * When using a tiny buffer (64 bytes), getdents may return only Valgrind FDs + * in a single call. The filtering code should detect this (new_size == 0) and + * automatically retry the syscall to get more entries until it finds client FDs + * or reaches EOF. This prevents the client from seeing empty results when + * Valgrind FDs get filtered out. + */ +static void test_retry_logic_with_small_buffer(void) +{ + int fd; + char buf[SMALL_BUF_SIZE]; + long nread; + struct linux_dirent *d; + + printf("retry_test_start\n"); + + fd = open("/proc/self/fd", O_RDONLY | O_DIRECTORY); + if (fd == -1) { + printf("retry_test_unavailable\n"); + return; + } + + /* + * Use raw getdents syscall with tiny 64-byte buffer. This forces multiple + * syscalls since each directory entry is typically 20+ bytes. Some calls + * may return only Valgrind FDs, which will trigger the retry mechanism. + */ + for (;;) { + nread = syscall(SYS_getdents, fd, buf, SMALL_BUF_SIZE); + + if (nread == -1) { + printf("retry_test_error\n"); + close(fd); + return; + } + + if (nread == 0) { + break; /* EOF - no more entries */ + } + + /* Print client FD entries found in this buffer (excluding . and ..) */ + for (size_t bpos = 0; bpos < nread;) { + d = (struct linux_dirent *)(buf + bpos); + if (strcmp(d->d_name, ".") != 0 && strcmp(d->d_name, "..") != 0) { + char *endptr; + long fd_num = strtol(d->d_name, &endptr, 10); + if (*endptr == '\0' && fd_num >= 0) { + printf("retry:%ld\n", fd_num); + } + } + bpos += d->d_reclen; + } + } + + close(fd); + printf("retry_test_end\n"); +} + +int main(void) +{ + /* Test 0: Retry logic with small buffer (tests the filtering refill mechanism) */ + test_retry_logic_with_small_buffer(); + + /* Test 1: /proc/self/fd should have Valgrind FDs filtered out */ + if (scan_proc_fd_directory("/proc/self/fd", NULL) == -1) { + return 0; /* Skip remaining tests if /proc/self/fd unavailable */ + } + + /* Test 2: /proc/self/fdinfo should have Valgrind FDs filtered out */ + scan_proc_fd_directory("/proc/self/fdinfo", "fdinfo"); + + /* Test 3: Regular directory should show all numbered files */ + DIR *d; + struct dirent *entry; + if (mkdir("testdir", 0755) == 0) { + FILE *f1 = fopen("testdir/1000", "w"); + FILE *f2 = fopen("testdir/1001", "w"); + if (f1) { fprintf(f1, "test"); fclose(f1); } + if (f2) { fprintf(f2, "test"); fclose(f2); } + + d = opendir("testdir"); + if (d != NULL) { + while ((entry = readdir(d)) != NULL) { + if (entry->d_name[0] != '.') { + char *endptr; + long num = strtol(entry->d_name, &endptr, 10); + if (*endptr == '\0' && num >= 1000) { + printf("regular:%ld\n", num); + } + } + } + closedir(d); + } + + unlink("testdir/1000"); + unlink("testdir/1001"); + rmdir("testdir"); + } + + + return 0; +} diff --git a/none/tests/getdents_filter.stderr.exp b/none/tests/getdents_filter.stderr.exp new file mode 100644 index 000000000..e69de29bb diff --git a/none/tests/getdents_filter.stdout.exp b/none/tests/getdents_filter.stdout.exp new file mode 100644 index 000000000..c228ee81f --- /dev/null +++ b/none/tests/getdents_filter.stdout.exp @@ -0,0 +1,16 @@ +retry_test_start +retry:0 +retry:1 +retry:2 +retry:3 +retry_test_end +0 +1 +2 +3 +fdinfo:0 +fdinfo:1 +fdinfo:2 +fdinfo:3 +regular:1000 +regular:1001 diff --git a/none/tests/getdents_filter.vgtest b/none/tests/getdents_filter.vgtest new file mode 100644 index 000000000..eb5bd6ceb --- /dev/null +++ b/none/tests/getdents_filter.vgtest @@ -0,0 +1,2 @@ +prog: getdents_filter +vgopts: -q