Menu
▾
▴
[Valgrind-developers] [valgrind] illumos syscalls: refactor *at directory fd checks
|
From: Paul F. <pa...@so...> - 2025-08-31 06:03:46
|
https://sourceware.org/cgit/valgrind/commit/?id=d2dc973f136d2c66e8320bd871a9acf832605662 commit d2dc973f136d2c66e8320bd871a9acf832605662 Author: Paul Floyd <pj...@wa...> Date: Sun Aug 31 08:02:00 2025 +0200 illumos syscalls: refactor *at directory fd checks Mainly to reduyce the amount of copied and pasted code. Slight improvements to messages to syscalls that have two directory fds. Diff: --- coregrind/m_syswrap/syswrap-solaris.c | 101 +++++---------------------- memcheck/tests/solaris/syscall_at.stderr.exp | 8 +-- 2 files changed, 21 insertions(+), 88 deletions(-) diff --git a/coregrind/m_syswrap/syswrap-solaris.c b/coregrind/m_syswrap/syswrap-solaris.c index f620e85bd7..a4f243f8ad 100644 --- a/coregrind/m_syswrap/syswrap-solaris.c +++ b/coregrind/m_syswrap/syswrap-solaris.c @@ -1812,18 +1812,8 @@ PRE(sys_linkat) PRE_MEM_RASCIIZ("linkat(path2)", ARG4); /* Be strict but ignore fd1/fd2 for absolute path1/path2. */ - if (fd1 != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(fd1, "linkat", tid, False)) { - SET_STATUS_Failure(VKI_EBADF); - } - if (fd2 != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG4, 1) - && ((HChar *) ARG4)[0] != '/' - && !ML_(fd_allowed)(fd2, "linkat", tid, False)) { - SET_STATUS_Failure(VKI_EBADF); - } + ML_(fd_at_check_allowed)(fd1, (const HChar*)ARG2, "linkat(efd)", tid, status); + ML_(fd_at_check_allowed)(fd2, (const HChar*)ARG4, "linkat(nfd)", tid, status); *flags |= SfMayBlock; } @@ -1844,11 +1834,7 @@ PRE(sys_symlinkat) PRE_MEM_RASCIIZ("symlinkat(path2)", ARG3); /* Be strict but ignore fd for absolute path2. */ - if (fd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG3, 1) - && ((HChar *) ARG3)[0] != '/' - && !ML_(fd_allowed)(fd, "symlinkat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(fd, (const HChar*)ARG3, "symlinkat", tid, status); *flags |= SfMayBlock; } @@ -2323,11 +2309,7 @@ PRE(sys_readlinkat) PRE_MEM_WRITE("readlinkat(buf)", ARG3, ARG4); /* Be strict but ignore dfd for absolute path. */ - if (dfd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(dfd, "readlinkat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(dfd, (const HChar*)ARG2, "readlinkat", tid, status); /* Handle the case where readlinkat is looking at /proc/self/path/a.out or /proc/<pid>/path/a.out. */ @@ -2386,11 +2368,7 @@ PRE(sys_frealpathat) PRE_MEM_WRITE("frealpathat(buf)", ARG3, ARG4); /* Be strict but ignore fd for absolute path. */ - if (fd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(fd, "frealpathat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(fd, (const HChar*)ARG2, "frealpathat", tid, status); } POST(sys_frealpathat) @@ -2529,11 +2507,7 @@ PRE(sys_faccessat) PRE_MEM_RASCIIZ("faccessat(path)", ARG2); /* Be strict but ignore fd for absolute path. */ - if (fd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(fd, "faccessat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(fd, (const HChar*)ARG2, "faccessat", tid, status); } PRE(sys_mknodat) @@ -2551,11 +2525,7 @@ PRE(sys_mknodat) PRE_MEM_RASCIIZ("mknodat(fname)", ARG2); /* Be strict but ignore fd for absolute path. */ - if (fd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(fd, "mknodat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(fd, (const HChar*)ARG2, "mknodat", tid, status); *flags |= SfMayBlock; } @@ -3593,11 +3563,7 @@ PRE(sys_fchownat) PRE_MEM_RASCIIZ("fchownat(path)", ARG2); /* Be strict but ignore fd for absolute path. */ - if (fd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(fd, "fchownat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(fd, (const HChar*)ARG2, "fchownat", tid, status); } PRE(sys_fdsync) @@ -4141,18 +4107,8 @@ PRE(sys_renameat) PRE_MEM_RASCIIZ("renameat(new)", ARG4); /* Be strict but ignore fromfd/tofd for absolute old/new. */ - if (fromfd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(fromfd, "renameat", tid, False)) { - SET_STATUS_Failure(VKI_EBADF); - } - if (tofd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG4, 1) - && ((HChar *) ARG4)[0] != '/' - && !ML_(fd_allowed)(tofd, "renameat", tid, False)) { - SET_STATUS_Failure(VKI_EBADF); - } + ML_(fd_at_check_allowed)(fromfd, (const HChar*)ARG2, "renameat(fromfd)", tid, status); + ML_(fd_at_check_allowed)(tofd, (const HChar*)ARG4, "renameat(tofd)", tid, status); } PRE(sys_unlinkat) @@ -4171,11 +4127,7 @@ PRE(sys_unlinkat) PRE_MEM_RASCIIZ("unlinkat(pathname)", ARG2); /* Be strict but ignore dfd for absolute pathname. */ - if (dfd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(dfd, "unlinkat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(dfd, (const HChar*)ARG2, "unlinkat", tid, status); } PRE(sys_fstatat) @@ -4199,11 +4151,7 @@ PRE(sys_fstatat) PRE_MEM_WRITE("fstatat(buf)", ARG3, sizeof(struct vki_stat)); /* Be strict but ignore fildes for absolute path. */ - if (fd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(fd, "fstatat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(fd, (const HChar*)ARG2, "fstatat", tid, status); } POST(sys_fstatat) @@ -4237,6 +4185,7 @@ PRE(sys_openat) PRE_MEM_RASCIIZ("openat(filename)", ARG2); + // @todo PJF use ML_(fd_at_check) and not return early here /* Be strict but ignore fildes for absolute pathname. */ if (fd != VKI_AT_FDCWD && ML_(safe_to_deref)((void *) ARG2, 1) @@ -5055,11 +5004,7 @@ PRE(sys_fchmodat) PRE_MEM_RASCIIZ("fchmodat(path)", ARG2); /* Be strict but ignore fd for absolute path. */ - if (fd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(fd, "fchmodat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(fd, (const HChar*)ARG2, "fchmodat", tid, status); } PRE(sys_mkdirat) @@ -5077,11 +5022,7 @@ PRE(sys_mkdirat) PRE_MEM_RASCIIZ("mkdirat(path)", ARG2); /* Be strict but ignore fd for absolute path. */ - if (fd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(fd, "mkdirat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(fd, (const HChar*)ARG2, "mkdirat", tid, status); } static void do_statvfs_post(struct vki_statvfs *stats, ThreadId tid) @@ -5273,11 +5214,7 @@ PRE(sys_utimesys) PRE_MEM_READ("utimesys(times)", ARG4, 2 * sizeof(vki_timespec_t)); /* Be strict but ignore fd for absolute path. */ - if (fd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG3, 1) - && ((HChar *) ARG3)[0] != '/' - && !ML_(fd_allowed)(fd, "utimesys", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(fd, (const HChar*)ARG3, "utimesys", tid, status); break; } default: @@ -5309,11 +5246,7 @@ PRE(sys_utimensat) PRE_MEM_READ("utimensat(times)", ARG3, 2 * sizeof(vki_timespec_t)); /* Be strict but ignore fd for absolute path. */ - if (fd != VKI_AT_FDCWD - && ML_(safe_to_deref)((void *) ARG2, 1) - && ((HChar *) ARG2)[0] != '/' - && !ML_(fd_allowed)(fd, "utimensat", tid, False)) - SET_STATUS_Failure(VKI_EBADF); + ML_(fd_at_check_allowed)(fd, (const HChar*)ARG2, "utimensat", tid, status); } #endif /* SOLARIS_UTIMENSAT_SYSCALL */ diff --git a/memcheck/tests/solaris/syscall_at.stderr.exp b/memcheck/tests/solaris/syscall_at.stderr.exp index b49fda289f..59815bfa98 100644 --- a/memcheck/tests/solaris/syscall_at.stderr.exp +++ b/memcheck/tests/solaris/syscall_at.stderr.exp @@ -1,12 +1,12 @@ -Warning: invalid file descriptor 159879507 in syscall linkat() -Warning: invalid file descriptor 159879508 in syscall linkat() +Warning: invalid file descriptor 159879507 in syscall linkat(efd)() +Warning: invalid file descriptor 159879508 in syscall linkat(nfd)() Warning: invalid file descriptor 646349138 in syscall symlinkat() Warning: invalid file descriptor 70680914 in syscall readlinkat() Warning: invalid file descriptor 68362578 in syscall faccessat() Warning: invalid file descriptor 70685266 in syscall fchownat() -Warning: invalid file descriptor 70717779 in syscall renameat() -Warning: invalid file descriptor 70717780 in syscall renameat() +Warning: invalid file descriptor 70717779 in syscall renameat(fromfd)() +Warning: invalid file descriptor 70717780 in syscall renameat(tofd)() Warning: invalid file descriptor 123765074 in syscall unlinkat() Warning: invalid file descriptor 1112625490 in syscall fstatat() Warning: invalid file descriptor 151224658 in syscall openat() |
