Serialize linux_flags writes on fd_lock

Several callers wrote fd_table[gfd].linux_flags under different locks
or none at all, so a concurrent fcntl(F_SETFL/F_SETFD) on fd_lock could
race a creator's bare assignment. sys_fcntl read the slot's type and
flags outside fd_lock and mutated them without revalidation, so a
close+reopen between the read and the write could update an unrelated
fd. This commit unifies both concerns under fd_lock.

fd_publish_linux_flags helper
  New fdtable helper takes fd_lock around a single linux_flags write.
  Replaces bare assignments in sys_timerfd_create, sys_eventfd,
  sys_signalfd, sys_inotify_init1, the FUSE dev mount, and fuse_open.
  fuse_dup_fd takes fd_lock once for both the source read and the
  destination write so the preserved-flags snapshot stays consistent
  with a racing F_SETFL on either fd. The result: every write to
  fd_table[*].linux_flags is now serialized on the same lock, with no
  fuse_lock<->fd_lock nesting introduced.

sys_fcntl snapshot-then-revalidate
  sys_fcntl now takes a single fd_snapshot at entry and uses it for
  F_GETFD, F_GETFL, and F_DUPFD source reads. F_SETFD and the FUSE /
  timerfd F_SETFL writers reacquire fd_lock and revalidate against
  fd_snap.generation before mutating linux_flags. fd_alloc bumps a
  monotonic generation counter per slot reuse, so close+reopen between
  snapshot and lock is caught and returns EBADF rather than mutating an
  unrelated fd.

The timerfd F_SETFL O_DIRECT EINVAL check moves inside the lock so a
stale-snapshot race cannot report EINVAL based on a fd that is no
longer a timerfd; the revalidation returns EBADF first instead.

A new test exercises the cross-cutting fd_lock RMW: F_SETFL stamps the
writable status bits, then F_SETFD toggles CLOEXEC, and F_GETFL must
still surface the status bits unperturbed.

Author: jserv

src/syscall/fd.c

@@ -218,9 +218,10 @@ int64_t sys_timerfd_create(int clockid, int flags)
      * fs/timerfd.c). Stamp O_RDWR into linux_flags so the F_GETFL branch
      * below can surface the access mode without re-deriving it.
      */
-    fd_table[gfd].linux_flags =
-        LINUX_O_RDWR | ((flags & LINUX_TFD_CLOEXEC) ? LINUX_O_CLOEXEC : 0) |
-        ((flags & LINUX_TFD_NONBLOCK) ? LINUX_O_NONBLOCK : 0);
+    fd_publish_linux_flags(
+        gfd, LINUX_O_RDWR |
+                 ((flags & LINUX_TFD_CLOEXEC) ? LINUX_O_CLOEXEC : 0) |
+                 ((flags & LINUX_TFD_NONBLOCK) ? LINUX_O_NONBLOCK : 0));
     return gfd;
 }
 
@@ -659,8 +660,8 @@ int64_t sys_eventfd2(unsigned int initval, int flags)
     eventfd_owner[gfd] = slot;
     pthread_mutex_unlock(&sfd_lock);
 
-    fd_table[gfd].linux_flags =
-        (flags & LINUX_EFD_CLOEXEC) ? LINUX_O_CLOEXEC : 0;
+    fd_publish_linux_flags(gfd,
+                           (flags & LINUX_EFD_CLOEXEC) ? LINUX_O_CLOEXEC : 0);
 
     /* If initial counter > 0, make the pipe readable so poll sees it */
     if (initval > 0) {
@@ -1083,8 +1084,8 @@ int64_t sys_signalfd4(guest_t *g,
     signalfd_state[slot].nonblock = (flags & LINUX_SFD_NONBLOCK) ? 1 : 0;
     pthread_mutex_unlock(&sfd_lock);
 
-    fd_table[gfd].linux_flags =
-        (flags & LINUX_SFD_CLOEXEC) ? LINUX_O_CLOEXEC : 0;
+    fd_publish_linux_flags(gfd,
+                           (flags & LINUX_SFD_CLOEXEC) ? LINUX_O_CLOEXEC : 0);
 
     return gfd;
 }

src/syscall/fdtable.c

@@ -404,6 +404,15 @@ int fd_get_type(int guest_fd)
     return type;
 }
 
+void fd_publish_linux_flags(int guest_fd, int linux_flags)
+{
+    if (!RANGE_CHECK(guest_fd, 0, FD_TABLE_SIZE))
+        return;
+    pthread_mutex_lock(&fd_lock);
+    fd_table[guest_fd].linux_flags = linux_flags;
+    pthread_mutex_unlock(&fd_lock);
+}
+
 /* Sized to cover all FD_* constants in abi.h plus a small headroom. Indexed
  * by type. Each slot defaults to NULL (no per-type cleanup). Modules that
  * own a type call fd_register_cleanup() at init time; dup and fork-restore

src/syscall/fs.c

@@ -668,7 +668,16 @@ int64_t sys_fcntl(guest_t *g, int fd, int cmd, uint64_t arg)
     if (!RANGE_CHECK(fd, 0, FD_TABLE_SIZE))
         return -LINUX_EBADF;
 
-    int fd_type = fd_table[fd].type;
+    /* Snapshot the slot under fd_lock once; readers use fd_snap below, and
+     * writers reacquire fd_lock and revalidate against fd_snap.generation
+     * so a close+reopen between the snapshot and the RMW returns EBADF
+     * instead of mutating an unrelated fd.
+     */
+    fd_entry_t fd_snap;
+    if (!fd_snapshot(fd, &fd_snap))
+        return -LINUX_EBADF;
+
+    int fd_type = fd_snap.type;
     bool fuse_fd = (fd_type == FD_FUSE_DEV || fd_type == FD_FUSE_FILE ||
                     fd_type == FD_FUSE_DIR);
 
@@ -681,7 +690,7 @@ int64_t sys_fcntl(guest_t *g, int fd, int cmd, uint64_t arg)
         if ((int) arg < 0) {
             return -LINUX_EINVAL;
         }
-        int dup_flags = fd_table[fd].linux_flags & ~LINUX_O_CLOEXEC;
+        int dup_flags = fd_snap.linux_flags & ~LINUX_O_CLOEXEC;
         if (cmd == 1030)
             dup_flags |= LINUX_O_CLOEXEC;
         int gfd = duplicate_guest_fd(fd, (int) arg, -1, false, dup_flags);
@@ -695,17 +704,28 @@ int64_t sys_fcntl(guest_t *g, int fd, int cmd, uint64_t arg)
         return gfd;
     }
     case 1: /* F_GETFD */
-        return (fd_table[fd].linux_flags & LINUX_O_CLOEXEC) ? LINUX_FD_CLOEXEC
-                                                            : 0;
+        return (fd_snap.linux_flags & LINUX_O_CLOEXEC) ? LINUX_FD_CLOEXEC : 0;
     case 2: /* F_SETFD */
+        /* Hold fd_lock across the read-modify-write so the CLOEXEC flip is
+         * atomic against a concurrent F_SETFL on the same shadow word and
+         * against any fd_lock-protected reader. Revalidate against the
+         * snapshot generation so a close+reopen returns EBADF.
+         */
+        pthread_mutex_lock(&fd_lock);
+        if (fd_table[fd].type == FD_CLOSED ||
+            fd_table[fd].generation != fd_snap.generation) {
+            pthread_mutex_unlock(&fd_lock);
+            return -LINUX_EBADF;
+        }
         if ((int) arg & LINUX_FD_CLOEXEC)
             fd_table[fd].linux_flags |= LINUX_O_CLOEXEC;
         else
             fd_table[fd].linux_flags &= ~LINUX_O_CLOEXEC;
+        pthread_mutex_unlock(&fd_lock);
         return 0;
     case 3: { /* F_GETFL */
         if (fuse_fd)
-            return fd_table[fd].linux_flags;
+            return fd_snap.linux_flags;
         /* Linux timerfd F_GETFL reports O_RDWR plus the writable status bits
          * the kernel honors. Surface only those bits from the shadow rather
          * than echoing arbitrary linux_flags bits so stray F_SETFL args
@@ -714,11 +734,8 @@ int64_t sys_fcntl(guest_t *g, int fd, int cmd, uint64_t arg)
          */
         if (fd_type == FD_TIMERFD)
             return LINUX_O_RDWR |
-                   (fd_table[fd].linux_flags &
+                   (fd_snap.linux_flags &
                     (LINUX_O_APPEND | LINUX_O_NONBLOCK | LINUX_O_NOATIME));
-        fd_entry_t snap;
-        if (!fd_snapshot(fd, &snap))
-            return -LINUX_EBADF;
         host_fd_ref_t host_ref;
         if (host_fd_ref_open(fd, &host_ref) < 0)
             return -LINUX_EBADF;
@@ -727,10 +744,10 @@ int64_t sys_fcntl(guest_t *g, int fd, int cmd, uint64_t arg)
         if (mac_fl < 0)
             return linux_errno();
         int linux_fl = mac_to_linux_status_flags(mac_fl);
-        if (snap.type == FD_REGULAR || snap.type == FD_DIR ||
-            snap.type == FD_PATH || snap.type == FD_URANDOM)
-            linux_fl = (linux_fl & ~O_ACCMODE) | (snap.linux_flags & 3);
-        linux_fl |= snap.linux_flags &
+        if (fd_snap.type == FD_REGULAR || fd_snap.type == FD_DIR ||
+            fd_snap.type == FD_PATH || fd_snap.type == FD_URANDOM)
+            linux_fl = (linux_fl & ~O_ACCMODE) | (fd_snap.linux_flags & 3);
+        linux_fl |= fd_snap.linux_flags &
                     (LINUX_O_PATH | LINUX_O_DIRECTORY | LINUX_O_NOFOLLOW |
                      LINUX_O_DIRECT | LINUX_O_LARGEFILE);
         return linux_fl;
@@ -742,7 +759,18 @@ int64_t sys_fcntl(guest_t *g, int fd, int cmd, uint64_t arg)
              * access mode in the Linux kernel, and without preserving it
              * here a stray F_SETFL(0) would silently flip an O_RDWR FUSE
              * shadow to O_RDONLY, surfacing the wrong mode through F_GETFL.
+             *
+             * Hold fd_lock across the read-modify-write so the update is
+             * atomic against a concurrent F_SETFD and any fd_lock-protected
+             * reader. Revalidate against the snapshot generation so a
+             * close+reopen returns EBADF.
              */
+            pthread_mutex_lock(&fd_lock);
+            if (fd_table[fd].type != fd_type ||
+                fd_table[fd].generation != fd_snap.generation) {
+                pthread_mutex_unlock(&fd_lock);
+                return -LINUX_EBADF;
+            }
             int preserved = fd_table[fd].linux_flags &
                             (LINUX_O_ACCMODE | LINUX_O_CLOEXEC | LINUX_O_PATH |
                              LINUX_O_DIRECTORY | LINUX_O_NOFOLLOW |
@@ -752,6 +780,7 @@ int64_t sys_fcntl(guest_t *g, int fd, int cmd, uint64_t arg)
                                            LINUX_O_PATH | LINUX_O_DIRECTORY |
                                            LINUX_O_NOFOLLOW | LINUX_O_DIRECT |
                                            LINUX_O_LARGEFILE));
+            pthread_mutex_unlock(&fd_lock);
             return 0;
         }
         /* Timerfd: kqueue host fd rejects fcntl(F_SETFL), so mirror Linux's
@@ -765,13 +794,22 @@ int64_t sys_fcntl(guest_t *g, int fd, int cmd, uint64_t arg)
          * matching how Linux F_SETFL drops them.
          */
         if (fd_type == FD_TIMERFD) {
-            if ((int) arg & LINUX_O_DIRECT)
-                return -LINUX_EINVAL;
             const int setfl_mask =
                 LINUX_O_APPEND | LINUX_O_NONBLOCK | LINUX_O_NOATIME;
+            pthread_mutex_lock(&fd_lock);
+            if (fd_table[fd].type != FD_TIMERFD ||
+                fd_table[fd].generation != fd_snap.generation) {
+                pthread_mutex_unlock(&fd_lock);
+                return -LINUX_EBADF;
+            }
+            if ((int) arg & LINUX_O_DIRECT) {
+                pthread_mutex_unlock(&fd_lock);
+                return -LINUX_EINVAL;
+            }
             fd_table[fd].linux_flags =
                 (fd_table[fd].linux_flags & ~setfl_mask) |
                 ((int) arg & setfl_mask);
+            pthread_mutex_unlock(&fd_lock);
             return 0;
         }
         host_fd_ref_t host_ref;

src/syscall/fuse.c

@@ -1329,8 +1329,11 @@ int fuse_proc_open(int linux_flags)
         errno = EMFILE;
         return -1;
     }
-    fd_table[guest_fd].linux_flags = linux_flags;
     pthread_mutex_unlock(&fuse_lock);
+    /* Publish under fd_lock so the write is on the same lock domain as
+     * sys_fcntl(F_SETFL/F_SETFD), not stranded behind fuse_lock.
+     */
+    fd_publish_linux_flags(guest_fd, linux_flags);
     return guest_fd;
 }
 
@@ -1897,8 +1900,11 @@ int64_t fuse_open_path(guest_t *g, const char *path, int linux_flags, int mode)
         fd_mark_closed(guest_fd);
         return -LINUX_EMFILE;
     }
-    fd_table[guest_fd].linux_flags = linux_flags;
     pthread_mutex_unlock(&fuse_lock);
+    /* Publish under fd_lock so the open's flags land on the same lock
+     * domain that sys_fcntl(F_SETFL/F_SETFD) uses.
+     */
+    fd_publish_linux_flags(guest_fd, linux_flags);
     return guest_fd;
 }
 
@@ -2607,16 +2613,24 @@ int fuse_dup_fd(int src_fd,
         }
     }
 
+    pthread_mutex_unlock(&fuse_lock);
+
     /* O_NONBLOCK is a file-status flag preserved by dup(2)/dup2(2); without
      * it a duplicated non-blocking FUSE fd would silently become blocking
      * because nothing else carries the flag forward.
+     *
+     * Take fd_lock once for both the source read and the destination write
+     * so the dup snapshot is consistent with any concurrent F_SETFL on the
+     * source and so the destination publish cannot be overwritten by an
+     * early racing F_SETFL on the new slot.
      */
+    pthread_mutex_lock(&fd_lock);
     int preserved_flags =
         fd_table[src_fd].linux_flags &
         (LINUX_O_PATH | LINUX_O_DIRECTORY | LINUX_O_NOFOLLOW | LINUX_O_DIRECT |
          LINUX_O_LARGEFILE | LINUX_O_NONBLOCK);
     fd_table[guest_fd].linux_flags = preserved_flags | linux_flags;
-    pthread_mutex_unlock(&fuse_lock);
+    pthread_mutex_unlock(&fd_lock);
     return guest_fd;
 }
 

src/syscall/inotify.c

@@ -402,7 +402,7 @@ int64_t sys_inotify_init1(int flags)
     memset(inst->watches, 0, sizeof(inst->watches));
     pthread_mutex_unlock(&inotify_lock);
 
-    fd_table[gfd].linux_flags = (flags & IN_CLOEXEC) ? LINUX_O_CLOEXEC : 0;
+    fd_publish_linux_flags(gfd, (flags & IN_CLOEXEC) ? LINUX_O_CLOEXEC : 0);
 
     return gfd;
 }

src/syscall/internal.h

@@ -109,6 +109,15 @@ int fd_snapshot_and_dup(int guest_fd, fd_entry_t *out);
  */
 int fd_get_type(int guest_fd);
 
+/* Publish linux_flags for a guest fd under fd_lock. Use after fd_alloc when
+ * the creating syscall needs to set linux_flags atomically with respect to a
+ * concurrent fcntl(F_SETFL/F_SETFD) on the same slot. The fd_alloc-then-
+ * publish window is small (the new gfd is not communicated to other threads
+ * until the syscall returns) but the lock removes the structural race and
+ * keeps every linux_flags writer on one lock domain.
+ */
+void fd_publish_linux_flags(int guest_fd, int linux_flags);
+
 /* Republish the EL1 urandom read fast-path bit for this fd from the current
  * fd_table type and access mode. Only readable /dev/urandom descriptors are
  * eligible for the bitmap.

tests/test-timerfd.c

@@ -225,6 +225,33 @@ int main(void)
             FAIL("timerfd_create");
     }
 
+    /* F_SETFD and F_SETFL touch the same linux_flags shadow but operate on
+     * disjoint bits. Toggling CLOEXEC via F_SETFD must not perturb the
+     * status bits surfaced by F_GETFL. Targets the new fd_lock-serialized
+     * RMW in both branches.
+     */
+    TEST("F_SETFD does not perturb F_GETFL status bits");
+    {
+        int fd = timerfd_create(CLOCK_MONOTONIC, 0);
+        if (fd >= 0) {
+            int wanted = O_APPEND | O_NONBLOCK | O_NOATIME;
+            EXPECT_TRUE(fcntl(fd, F_SETFL, wanted) == 0, "F_SETFL failed");
+
+            EXPECT_TRUE(fcntl(fd, F_SETFD, FD_CLOEXEC) == 0,
+                        "F_SETFD set failed");
+            int fl = fcntl(fd, F_GETFL);
+            EXPECT_TRUE(fl >= 0 && (fl & wanted) == wanted,
+                        "F_SETFD CLOEXEC perturbed status bits");
+
+            EXPECT_TRUE(fcntl(fd, F_SETFD, 0) == 0, "F_SETFD clear failed");
+            fl = fcntl(fd, F_GETFL);
+            EXPECT_TRUE(fl >= 0 && (fl & wanted) == wanted,
+                        "F_SETFD clear perturbed status bits");
+            close(fd);
+        } else
+            FAIL("timerfd_create");
+    }
+
     /* dup must carry the linux_flags shadow's NONBLOCK bit through; without
      * the preserved-mask fix the new fd's F_GETFL would report blocking.
      * (The timerfd slot table is keyed by guest_fd with no alias support,