Add fd_allowed and POST_newFd_RES to all syscalls that use or return fds

This makes sure all file descriptors that take a file descriptor check
that the file descriptor is valid. Also makes sure that the
--modify-fds=high option affects all sycalls that return a file
descriptor.

https://bugs.kde.org/show_bug.cgi?id=493430

diff --git a/NEWS b/NEWS
index 636bf61dd90dc2944084d9f3c483604ec15e375f..00c785dfdb9cfba135ac7d662f7a7a7da2661aee 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -31,6 +31,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 
 338803  Handling of dwz debug alt files or cross-CU is broken
 418756  MAP_FIXED_NOREPLACE mmap flag unsupported
+493430  Review all syscalls that use or return (new) file descriptors
 493434  Add --track-fds=bad mode (no "leak" tracking)
 501741  syscall cachestat not wrapped
 502359  Add --modify-fds=yes option

diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c
index 7ac86fbbc02f1987132770635422f09736d2c739..c7d58bc10601d8a9595ea7078af855e3c54de676 100644 (file)
--- a/coregrind/m_syswrap/syswrap-generic.c
+++ b/coregrind/m_syswrap/syswrap-generic.c
@@ -2944,6 +2944,8 @@ PRE(sys_fsync)
    *flags |= SfMayBlock;
    PRINT("sys_fsync ( %" FMT_REGWORD "u )", ARG1);
    PRE_REG_READ1(long, "fsync", unsigned int, fd);
+   if ( !ML_(fd_allowed)(ARG1, "fsync", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_fdatasync)
@@ -2951,6 +2953,8 @@ PRE(sys_fdatasync)
    *flags |= SfMayBlock;
    PRINT("sys_fdatasync ( %" FMT_REGWORD "u )", ARG1);
    PRE_REG_READ1(long, "fdatasync", unsigned int, fd);
+   if ( !ML_(fd_allowed)(ARG1, "fdatasync", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_msync)
@@ -3215,6 +3219,8 @@ PRE(sys_fstatfs)
    PRE_REG_READ2(long, "fstatfs",
                  unsigned int, fd, struct statfs *, buf);
    PRE_MEM_WRITE( "fstatfs(buf)", ARG2, sizeof(struct vki_statfs) );
+   if ( !ML_(fd_allowed)(ARG1, "fstatfs", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 POST(sys_fstatfs)
@@ -3230,6 +3236,8 @@ PRE(sys_fstatfs64)
    PRE_REG_READ3(long, "fstatfs64",
                  unsigned int, fd, vki_size_t, size, struct statfs64 *, buf);
    PRE_MEM_WRITE( "fstatfs64(buf)", ARG3, ARG2 );
+   if ( !ML_(fd_allowed)(ARG1, "fstatfs64", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 POST(sys_fstatfs64)
 {
@@ -3288,6 +3296,8 @@ PRE(sys_flock)
    *flags |= SfMayBlock;
    PRINT("sys_flock ( %" FMT_REGWORD "u, %" FMT_REGWORD "u )", ARG1, ARG2 );
    PRE_REG_READ2(long, "flock", unsigned int, fd, unsigned int, operation);
+   if ( !ML_(fd_allowed)(ARG1, "flock", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 // Pre_read a char** argument.
@@ -3818,6 +3828,8 @@ PRE(sys_fchdir)
    FUSE_COMPATIBLE_MAY_BLOCK();
    PRINT("sys_fchdir ( %" FMT_REGWORD "u )", ARG1);
    PRE_REG_READ1(long, "fchdir", unsigned int, fd);
+   if ( !ML_(fd_allowed)(ARG1, "fchdir", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_fchown)
@@ -3827,6 +3839,8 @@ PRE(sys_fchown)
          FMT_REGWORD "u )", ARG1, ARG2, ARG3);
    PRE_REG_READ3(long, "fchown",
                  unsigned int, fd, vki_uid_t, owner, vki_gid_t, group);
+   if ( !ML_(fd_allowed)(ARG1, "fchown", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_fchmod)
@@ -3834,6 +3848,8 @@ PRE(sys_fchmod)
    FUSE_COMPATIBLE_MAY_BLOCK();
    PRINT("sys_fchmod ( %" FMT_REGWORD "u, %" FMT_REGWORD "u )", ARG1, ARG2);
    PRE_REG_READ2(long, "fchmod", unsigned int, fildes, vki_mode_t, mode);
+   if ( !ML_(fd_allowed)(ARG1, "fchmod", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 #if !defined(VGP_nanomips_linux) && !defined (VGO_freebsd)
@@ -3910,6 +3926,8 @@ PRE(sys_ftruncate)
    *flags |= SfMayBlock;
    PRINT("sys_ftruncate ( %" FMT_REGWORD "u, %" FMT_REGWORD "u )", ARG1, ARG2);
    PRE_REG_READ2(long, "ftruncate", unsigned int, fd, unsigned long, length);
+   if ( !ML_(fd_allowed)(ARG1, "ftruncate", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_truncate)
@@ -3936,6 +3954,8 @@ PRE(sys_ftruncate64)
    PRE_REG_READ2(long, "ftruncate64",
                  unsigned int,fd, UWord,length);
 #endif
+   if ( !ML_(fd_allowed)(ARG1, "ftruncate64", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_truncate64)
@@ -4801,6 +4821,9 @@ PRE(sys_poll)
       PRE_MEM_READ( "poll(ufds.fd)",
                     (Addr)(&ufds[i].fd), sizeof(ufds[i].fd) );
       if (ML_(safe_to_deref)(&ufds[i].fd, sizeof(ufds[i].fd)) && ufds[i].fd >= 0) {
+         if (!ML_(fd_allowed)(ufds[i].fd, "poll(ufds.fd)", tid, False)) {
+            /* do nothing? Just let fd_allowed produce a warning? */
+         }
          PRE_MEM_READ( "poll(ufds.events)",
                        (Addr)(&ufds[i].events), sizeof(ufds[i].events) );
       }

diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 552fceee8d04a31d36ba7d78d171137762d1b510..572a42925da8ada4b761a1a92bb5e2c1f6bb4e7b 100644 (file)
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -2518,6 +2518,8 @@ PRE(sys_fadvise64)
    PRE_REG_READ5(long, "fadvise64",
                  int, fd, vki_u32, MERGE64_FIRST(offset), vki_u32, MERGE64_SECOND(offset),
                  vki_size_t, len, int, advice);
+   if ( !ML_(fd_allowed)(SARG1, "fadvise64", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_fadvise64_64)
@@ -2527,6 +2529,8 @@ PRE(sys_fadvise64_64)
    PRE_REG_READ6(long, "fadvise64_64",
                  int, fd, vki_u32, MERGE64_FIRST(offset), vki_u32, MERGE64_SECOND(offset),
                  vki_u32, MERGE64_FIRST(len), vki_u32, MERGE64_SECOND(len), int, advice);
+   if ( !ML_(fd_allowed)(SARG1, "fadvise64_64", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 }
 
 /* ---------------------------------------------------------------------
@@ -2861,6 +2865,8 @@ PRE(sys_fanotify_mark)
                  int, dfd, const char *, pathname);
    if (ARG5)
       PRE_MEM_RASCIIZ( "fanotify_mark(path)", ARG5);
+   if ( !ML_(fd_allowed)(SARG1, "fanotify_mark", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
 #else
 #  error Unexpected word size
 #endif
@@ -2897,6 +2903,7 @@ PRE(sys_inotify_init1)
 POST(sys_inotify_init1)
 {
    vg_assert(SUCCESS);
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "inotify_init", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -2943,6 +2950,7 @@ PRE(sys_mq_open)
 POST(sys_mq_open)
 {
    vg_assert(SUCCESS);
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "mq_open", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -3373,6 +3381,7 @@ PRE(sys_timerfd_create)
 }
 POST(sys_timerfd_create)
 {
+   POST_newFd_RES;
    if (linux_kernel_2_6_22())
    {
       /* 2.6.22 kernel: timerfd system call. */
@@ -3615,6 +3624,9 @@ PRE(sys_fchown16)
          FMT_REGWORD "u )", ARG1, ARG2, ARG3);
    PRE_REG_READ3(long, "fchown16",
                  unsigned int, fd, vki_old_uid_t, owner, vki_old_gid_t, group);
+   if ( !ML_(fd_allowed)(ARG1, "fchown16", tid, False) )
+      SET_STATUS_Failure( VKI_EBADF );
+
 }
 
 /* ---------------------------------------------------------------------
@@ -3709,6 +3721,9 @@ PRE(sys_fgetxattr)
                  int, fd, char *, name, void *, value, vki_size_t, size);
    PRE_MEM_RASCIIZ( "fgetxattr(name)", ARG2 );
    PRE_MEM_WRITE( "fgetxattr(value)", ARG3, ARG4 );
+   if ( !ML_(fd_allowed)(SARG1, "fgetxattr", tid, False) )
+     SET_STATUS_Failure( VKI_EBADF );
+
 }
 POST(sys_fgetxattr)
 {
@@ -3756,6 +3771,9 @@ PRE(sys_flistxattr)
    PRE_REG_READ3(ssize_t, "flistxattr",
                  int, fd, char *, list, vki_size_t, size);
    PRE_MEM_WRITE( "flistxattr(list)", ARG2, ARG3 );
+   if ( !ML_(fd_allowed)(ARG1, "flistxattr", tid, False) )
+     SET_STATUS_Failure( VKI_EBADF );
+
 }
 POST(sys_flistxattr)
 {
@@ -3789,6 +3807,9 @@ PRE(sys_fremovexattr)
    PRINT("sys_fremovexattr ( %ld, %#" FMT_REGWORD "x )", SARG1, ARG2);
    PRE_REG_READ2(long, "fremovexattr", int, fd, char *, name);
    PRE_MEM_RASCIIZ( "fremovexattr(name)", ARG2 );
+   if ( !ML_(fd_allowed)(SARG1, "fremovexattr", tid, False) )
+     SET_STATUS_Failure( VKI_EBADF );
+
 }
 
 /* ---------------------------------------------------------------------
@@ -4151,6 +4172,7 @@ PRE(sys_perf_event_open)
 POST(sys_perf_event_open)
 {
    vg_assert(SUCCESS);
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "perf_event_open", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -4229,6 +4251,7 @@ PRE(sys_memfd_create)
 POST(sys_memfd_create)
 {
    vg_assert(SUCCESS);
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "memfd_create", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -4255,6 +4278,7 @@ POST(sys_landlock_create_ruleset)
 {
    /* Returns either the abi version or a file descriptor.  */
    if (ARG3 != VKI_LANDLOCK_CREATE_RULESET_VERSION) {
+      POST_newFd_RES;
       if (!ML_(fd_allowed)(RES, "landlock_create_ruleset", tid, True)) {
          VG_(close)(RES);
          SET_STATUS_Failure( VKI_EMFILE );
@@ -4295,6 +4319,7 @@ PRE(sys_memfd_secret)
 POST(sys_memfd_secret)
 {
    vg_assert(SUCCESS);
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "memfd_secret", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -4626,6 +4651,7 @@ PRE(sys_signalfd)
 }
 POST(sys_signalfd)
 {
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "signalfd", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -4647,6 +4673,7 @@ PRE(sys_signalfd4)
 }
 POST(sys_signalfd4)
 {
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "signalfd4", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -6143,6 +6170,8 @@ PRE(sys_fchownat)
                  int, dfd, const char *, path,
                  vki_uid_t, owner, vki_gid_t, group);
    PRE_MEM_RASCIIZ( "fchownat(path)", ARG2 );
+   if ( !ML_(fd_allowed)(SARG1, "fchownat", tid, False) )
+     SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_futimesat)
@@ -6156,6 +6185,9 @@ PRE(sys_futimesat)
       PRE_MEM_RASCIIZ( "futimesat(filename)", ARG2 );
    if (ARG3 != 0)
       PRE_MEM_READ( "futimesat(tvp)", ARG3, 2 * sizeof(struct vki_timeval) );
+   if ( !ML_(fd_allowed)(SARG1, "futimesat", tid, False) )
+     SET_STATUS_Failure( VKI_EBADF );
+
 }
 
 PRE(sys_utimensat)
@@ -6345,6 +6377,9 @@ PRE(sys_fchmodat)
    PRE_REG_READ3(long, "fchmodat",
                  int, dfd, const char *, path, vki_mode_t, mode);
    PRE_MEM_RASCIIZ( "fchmodat(path)", ARG2 );
+   if ( !ML_(fd_allowed)(SARG1, "fchmodat", tid, False) )
+     SET_STATUS_Failure( VKI_EBADF );
+
 }
 
 PRE(sys_cachestat)
@@ -6377,6 +6412,8 @@ PRE(sys_fchmodat2)
                  int, dfd, const char *, path, vki_mode_t, mode,
                  unsigned int, flags);
    PRE_MEM_RASCIIZ( "fchmodat2(pathname)", ARG2 );
+   if ( !ML_(fd_allowed)(SARG1, "fchmodat2", tid, False) )
+     SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_faccessat)
@@ -6387,6 +6424,9 @@ PRE(sys_faccessat)
    PRE_REG_READ3(long, "faccessat",
                  int, dfd, const char *, pathname, int, mode);
    PRE_MEM_RASCIIZ( "faccessat(pathname)", ARG2 );
+   if ( !ML_(fd_allowed)(SARG1, "faccessat", tid, False) )
+     SET_STATUS_Failure( VKI_EBADF );
+
 }
 
 PRE(sys_faccessat2)
@@ -6397,6 +6437,8 @@ PRE(sys_faccessat2)
    PRE_REG_READ4(long, "faccessat2",
                  int, dfd, const char *, pathname, int, mode, int, flags);
    PRE_MEM_RASCIIZ( "faccessat2(pathname)", ARG2 );
+   if ( !ML_(fd_allowed)(SARG1, "faccessat2", tid, False) )
+     SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_name_to_handle_at)
@@ -6441,6 +6483,7 @@ PRE(sys_open_by_handle_at)
 POST(sys_open_by_handle_at)
 {
    vg_assert(SUCCESS);
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "open_by_handle_at", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -7014,6 +7057,8 @@ PRE(sys_finit_module)
    PRE_REG_READ3(long, "finit_module",
                  int, fd, const char *, params, int, flags);
    PRE_MEM_RASCIIZ("finit_module(params)", ARG2);
+   if ( !ML_(fd_allowed)(ARG1, "finit_module", tid, False) )
+     SET_STATUS_Failure( VKI_EBADF );
 }
 
 PRE(sys_delete_module)
@@ -7284,6 +7329,7 @@ POST(sys_fcntl)
 {
    vg_assert(SUCCESS);
    if (ARG2 == VKI_F_DUPFD) {
+      POST_newFd_RES;
       if (!ML_(fd_allowed)(RES, "fcntl(DUPFD)", tid, True)) {
          VG_(close)(RES);
          SET_STATUS_Failure( VKI_EMFILE );
@@ -7293,6 +7339,7 @@ POST(sys_fcntl)
       }
    }
    else if (ARG2 == VKI_F_DUPFD_CLOEXEC) {
+      POST_newFd_RES;
       if (!ML_(fd_allowed)(RES, "fcntl(DUPFD_CLOEXEC)", tid, True)) {
          VG_(close)(RES);
          SET_STATUS_Failure( VKI_EMFILE );
@@ -7398,6 +7445,7 @@ POST(sys_fcntl64)
 {
    vg_assert(SUCCESS);
    if (ARG2 == VKI_F_DUPFD) {
+      POST_newFd_RES;
       if (!ML_(fd_allowed)(RES, "fcntl64(DUPFD)", tid, True)) {
          VG_(close)(RES);
          SET_STATUS_Failure( VKI_EMFILE );
@@ -7407,6 +7455,7 @@ POST(sys_fcntl64)
       }
    }
    else if (ARG2 == VKI_F_DUPFD_CLOEXEC) {
+      POST_newFd_RES;
       if (!ML_(fd_allowed)(RES, "fcntl64(DUPFD_CLOEXEC)", tid, True)) {
          VG_(close)(RES);
          SET_STATUS_Failure( VKI_EMFILE );
@@ -13521,6 +13570,7 @@ POST(sys_bpf)
       case VKI_BPF_MAP_GET_FD_BY_ID:
       case VKI_BPF_BTF_GET_FD_BY_ID:
       case VKI_BPF_RAW_TRACEPOINT_OPEN:
+         POST_newFd_RES;
          if (!ML_(fd_allowed)(RES, "bpf", tid, True)) {
             VG_(close)(RES);
             SET_STATUS_Failure(VKI_EMFILE);
@@ -13543,6 +13593,7 @@ POST(sys_bpf)
          break;
       case VKI_BPF_PROG_LOAD:
          /* Return a file descriptor for loaded program, write into log_buf. */
+         POST_newFd_RES;
          if (!ML_(fd_allowed)(RES, "bpf", tid, True)) {
             VG_(close)(RES);
             SET_STATUS_Failure(VKI_EMFILE);
@@ -13571,6 +13622,7 @@ POST(sys_bpf)
          break;
       case VKI_BPF_BTF_LOAD:
          /* Return a file descriptor for BTF data, write into btf_log_buf. */
+         POST_newFd_RES;
          if (!ML_(fd_allowed)(RES, "bpf", tid, True)) {
             VG_(close)(RES);
             SET_STATUS_Failure(VKI_EMFILE);
@@ -13729,6 +13781,7 @@ PRE(sys_io_uring_setup)
 POST(sys_io_uring_setup)
 {
    vg_assert(SUCCESS);
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "io_uring_setup", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -14104,6 +14157,7 @@ PRE(sys_openat2)
 POST(sys_openat2)
 {
    vg_assert(SUCCESS);
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "openat2", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -14120,6 +14174,7 @@ PRE(sys_pidfd_open)
 
 POST(sys_pidfd_open)
 {
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "pidfd", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -14138,6 +14193,7 @@ PRE(sys_pidfd_getfd)
 POST(sys_pidfd_getfd)
 {
    vg_assert(SUCCESS);
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "pidfd_getfd", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -14167,6 +14223,7 @@ PRE(sys_open_tree)
 
 POST(sys_open_tree)
 {
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "open_tree", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -14219,6 +14276,7 @@ PRE(sys_fsopen)
 
 POST(sys_fsopen)
 {
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "fsopen", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -14239,6 +14297,7 @@ PRE(sys_fsmount)
 
 POST(sys_fsmount)
 {
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "fsmount", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -14284,6 +14343,7 @@ PRE(sys_fspick)
 
 POST(sys_fspick)
 {
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "fspick", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );
@@ -14304,6 +14364,7 @@ PRE(sys_userfaultfd)
 POST(sys_userfaultfd)
 {
    vg_assert(SUCCESS);
+   POST_newFd_RES;
    if (!ML_(fd_allowed)(RES, "userfaultfd", tid, True)) {
       VG_(close)(RES);
       SET_STATUS_Failure( VKI_EMFILE );