diff --git a/src/syscall/fs.c b/src/syscall/fs.c index 30dbb0d1..45f538cc 100644 --- a/src/syscall/fs.c +++ b/src/syscall/fs.c @@ -1017,6 +1017,130 @@ int64_t sys_dup3(int oldfd, int newfd, int linux_flags) return newfd; } +/* Translate a Linux struct flock (aarch64) at `arg` to macOS layout, run + * fcntl(host_fd, mac_cmd, ...), and for a GETLK command write the result + * back translated to Linux layout. Shared by the traditional (F_GETLK/ + * F_SETLK/F_SETLKW) and OFD (F_OFD_GETLK/F_OFD_SETLK/F_OFD_SETLKW) lock + * commands, which differ only in the macOS cmd values and in how l_pid is + * reported back for a GETLK conflict. + * + * Linux aarch64 layout: {short l_type, short l_whence, + * long l_start, long l_len, int l_pid, pad[4]} + * macOS layout: {off_t l_start, off_t l_len, pid_t l_pid, + * short l_type, short l_whence} + * Use guest_read/guest_write (not guest_ptr) to safely handle structs that + * span 2MiB page table block boundaries. + */ +static int64_t fcntl_flock_op(guest_t *g, + host_fd_ref_t *host_ref, + uint64_t arg, + int mac_cmd, + bool is_getlk, + bool is_ofd) +{ + uint8_t lflock[32]; /* Linux struct flock is 32 bytes on aarch64 */ + if (guest_read_small(g, arg, lflock, sizeof(lflock)) < 0) + return -LINUX_EFAULT; + + int16_t l_type, l_whence; + int64_t l_start, l_len; + int32_t l_pid; + memcpy(&l_type, lflock + 0, 2); + memcpy(&l_whence, lflock + 2, 2); + memcpy(&l_start, lflock + 8, 8); /* offset 8 due to padding */ + memcpy(&l_len, lflock + 16, 8); + memcpy(&l_pid, lflock + 24, 4); + + /* Linux rejects F_OFD_GETLK/SETLK/SETLKW requests with a nonzero l_pid: + * OFD locks are owned by the open file description, not a process, so + * the field is reserved on input (fs/locks.c fcntl_getlk/fcntl_setlk + * both `return -EINVAL` on a nonzero request l_pid for these commands). + */ + if (is_ofd && l_pid != 0) + return -LINUX_EINVAL; + + /* l_type constants differ between Linux and macOS/BSD: + * Linux: F_RDLCK=0, F_WRLCK=1, F_UNLCK=2 + * macOS: F_RDLCK=1, F_UNLCK=2, F_WRLCK=3 + * Passing the Linux value straight through makes a Linux F_RDLCK (0) an + * invalid type on macOS, which fcntl() rejects with EINVAL. This is the + * lock POSIX databases (e.g. SQLite) take first, so it must map. + */ + short mac_type; + switch (l_type) { + case 0: /* LINUX_F_RDLCK */ + mac_type = F_RDLCK; + break; + case 1: /* LINUX_F_WRLCK */ + mac_type = F_WRLCK; + break; + case 2: /* LINUX_F_UNLCK */ + mac_type = F_UNLCK; + break; + default: + return -LINUX_EINVAL; + } + + struct flock mac_fl = { + .l_start = l_start, + .l_len = l_len, + .l_pid = 0, + .l_type = mac_type, + .l_whence = l_whence, /* SEEK_SET=0, SEEK_CUR=1, SEEK_END=2 same */ + }; + + if (fcntl(host_ref->fd, mac_cmd, &mac_fl) < 0) + return linux_errno(); + + if (!is_getlk) + return 0; + + /* Map macOS l_type back to Linux constants (see above). */ + int16_t rt; + switch (mac_fl.l_type) { + case F_RDLCK: + rt = 0; /* LINUX_F_RDLCK */ + break; + case F_WRLCK: + rt = 1; /* LINUX_F_WRLCK */ + break; + default: + rt = 2; /* LINUX_F_UNLCK */ + break; + } + int16_t rw = mac_fl.l_whence; + int64_t rs = mac_fl.l_start, rl = mac_fl.l_len; + int32_t rp; + if (is_ofd) { + /* OFD locks are owned by the open file description, not a single + * process, so Linux always reports l_pid=-1 on a conflicting + * F_OFD_GETLK lock instead of leaking a host PID to the guest. + */ + rp = (rt == 2) ? 0 : -1; + } else if (rt == 2) { + rp = (int32_t) mac_fl.l_pid; /* F_UNLCK: no conflict to translate */ + } else { + /* mac_fl.l_pid is a raw host PID, meaningless to guest code that + * treats it as a real PID (e.g. a liveness check via kill(pid, 0)). + * Translate it to the conflicting process's guest PID when it is + * part of this guest's fork family; fall back to the host PID only + * when the lock holder cannot be resolved (e.g. an unrelated host + * process), since no guest identity exists for it to report. + */ + int64_t gpid = proc_host_to_guest_pid((pid_t) mac_fl.l_pid); + rp = (gpid > 0) ? (int32_t) gpid : (int32_t) mac_fl.l_pid; + } + memset(lflock, 0, sizeof(lflock)); + memcpy(lflock + 0, &rt, 2); + memcpy(lflock + 2, &rw, 2); + memcpy(lflock + 8, &rs, 8); + memcpy(lflock + 16, &rl, 8); + memcpy(lflock + 24, &rp, 4); + if (guest_write_small(g, arg, lflock, sizeof(lflock)) < 0) + return -LINUX_EFAULT; + return 0; +} + int64_t sys_fcntl(guest_t *g, int fd, int cmd, uint64_t arg) { if (!RANGE_CHECK(fd, 0, FD_TABLE_SIZE)) @@ -1196,95 +1320,28 @@ int64_t sys_fcntl(guest_t *g, int fd, int cmd, uint64_t arg) host_fd_ref_t host_ref; if (host_fd_ref_open(fd, &host_ref) < 0) return -LINUX_EBADF; - /* Translate Linux struct flock (aarch64) to macOS struct flock. Linux - * aarch64 layout: {short l_type, short l_whence, - * long l_start, long l_len, int l_pid, pad[4]} - * macOS layout: {off_t l_start, off_t l_len, pid_t l_pid, - * short l_type, short l_whence} - * Use guest_read/guest_write (not guest_ptr) to safely handle structs - * that span 2MiB page table block boundaries. - */ - uint8_t lflock[32]; /* Linux struct flock is 32 bytes on aarch64 */ - if (guest_read_small(g, arg, lflock, sizeof(lflock)) < 0) - return -LINUX_EFAULT; - - /* Read Linux flock fields */ - int16_t l_type, l_whence; - int64_t l_start, l_len; - memcpy(&l_type, lflock + 0, 2); - memcpy(&l_whence, lflock + 2, 2); - memcpy(&l_start, lflock + 8, 8); /* offset 8 due to padding */ - memcpy(&l_len, lflock + 16, 8); - - /* l_type constants differ between Linux and macOS/BSD: - * Linux: F_RDLCK=0, F_WRLCK=1, F_UNLCK=2 - * macOS: F_RDLCK=1, F_UNLCK=2, F_WRLCK=3 - * Passing the Linux value straight through makes a Linux F_RDLCK (0) an - * invalid type on macOS, which fcntl() rejects with EINVAL. This is the - * lock POSIX databases (e.g. SQLite) take first, so it must map. - */ - short mac_type; - switch (l_type) { - case 0: /* LINUX_F_RDLCK */ - mac_type = F_RDLCK; - break; - case 1: /* LINUX_F_WRLCK */ - mac_type = F_WRLCK; - break; - case 2: /* LINUX_F_UNLCK */ - mac_type = F_UNLCK; - break; - default: - host_fd_ref_close(&host_ref); - return -LINUX_EINVAL; - } - - struct flock mac_fl = { - .l_start = l_start, - .l_len = l_len, - .l_pid = 0, - .l_type = mac_type, - .l_whence = l_whence, /* SEEK_SET=0, SEEK_CUR=1, SEEK_END=2 same */ - }; - int mac_cmd = (cmd == 5) ? F_GETLK : (cmd == 6) ? F_SETLK : F_SETLKW; - if (fcntl(host_ref.fd, mac_cmd, &mac_fl) < 0) { - host_fd_ref_close(&host_ref); - return linux_errno(); - } - - /* For F_GETLK, write back the result */ - if (cmd == 5) { - /* Map macOS l_type back to Linux constants (see above). */ - int16_t rt; - switch (mac_fl.l_type) { - case F_RDLCK: - rt = 0; /* LINUX_F_RDLCK */ - break; - case F_WRLCK: - rt = 1; /* LINUX_F_WRLCK */ - break; - default: - rt = 2; /* LINUX_F_UNLCK */ - break; - } - int16_t rw = mac_fl.l_whence; - int64_t rs = mac_fl.l_start, rl = mac_fl.l_len; - int32_t rp = mac_fl.l_pid; - memset(lflock, 0, sizeof(lflock)); - memcpy(lflock + 0, &rt, 2); - memcpy(lflock + 2, &rw, 2); - memcpy(lflock + 8, &rs, 8); - memcpy(lflock + 16, &rl, 8); - memcpy(lflock + 24, &rp, 4); - if (guest_write_small(g, arg, lflock, sizeof(lflock)) < 0) { - host_fd_ref_close(&host_ref); - return -LINUX_EFAULT; - } - } + int64_t rc = + fcntl_flock_op(g, &host_ref, arg, mac_cmd, cmd == 5, false); host_fd_ref_close(&host_ref); - return 0; + return rc; + } +#if defined(F_OFD_GETLK) && defined(F_OFD_SETLK) && defined(F_OFD_SETLKW) + case 36: /* F_OFD_GETLK */ + case 37: /* F_OFD_SETLK */ + case 38: { /* F_OFD_SETLKW */ + host_fd_ref_t host_ref; + if (host_fd_ref_open(fd, &host_ref) < 0) + return -LINUX_EBADF; + int mac_cmd = (cmd == 36) ? F_OFD_GETLK + : (cmd == 37) ? F_OFD_SETLK + : F_OFD_SETLKW; + int64_t rc = + fcntl_flock_op(g, &host_ref, arg, mac_cmd, cmd == 36, true); + host_fd_ref_close(&host_ref); + return rc; } +#endif case 8: { /* F_SETOWN */ /* SIGIO/SIGURG delivery owner. The arg is a signed value passed by * value: pid > 0 targets a process, pid < 0 targets a process group, diff --git a/src/syscall/proc.c b/src/syscall/proc.c index dcdf9944..407c8aa6 100644 --- a/src/syscall/proc.c +++ b/src/syscall/proc.c @@ -514,6 +514,35 @@ static void registry_parse_cb(char *rec, void *vctx) c->entries[idx].pgid = (int64_t) pg; } +typedef struct { + pid_t target; + int64_t guest_pid; + bool found; +} registry_find_ctx_t; + +/* Locate @target's guest pid without registry_parse_cb's per-record + * kill(2) liveness probe: that check exists to build a filtered live- + * membership list for group-signal delivery, but a host_pid ->guest_pid + * lookup is only ever done for a pid the caller just observed to be alive + * (e.g. it holds a conflicting file lock right now), so it is redundant + * here. proc_host_to_guest_pid still verifies the match via proc_pidpath + * to guard against the pid having been recycled. + */ +static void registry_find_by_host_cb(char *rec, void *vctx) +{ + registry_find_ctx_t *c = vctx; + long hp; + long long gp, pg; + if (sscanf(rec, "%ld %lld %lld", &hp, &gp, &pg) != 3) + return; + if (hp <= 0 || hp > INT_MAX || pg < 0 || pg > INT_MAX) + return; + if ((pid_t) hp != c->target) + return; + c->guest_pid = (int64_t) gp; + c->found = true; +} + /* Parse the whole registry from @fd (caller holds an flock) into @entries, * keeping one record per live host pid. * @@ -805,6 +834,46 @@ int proc_get_namespace_targets(proc_signal_target_t *out, return count; } +int64_t proc_host_to_guest_pid(pid_t host_pid) +{ + pthread_mutex_lock(&pid_lock); + proc_entry_t *entry = proc_find_host_entry(host_pid); + int64_t result = entry ? entry->guest_pid : -1; + pthread_mutex_unlock(&pid_lock); + if (result != -1) + return result; + + char path[PATH_MAX]; + if (!process_registry_path(path, sizeof(path))) + return -1; + int fd = open(path, O_RDONLY | O_NOFOLLOW | O_CLOEXEC); + if (fd < 0) + return -1; + if (flock_retry(fd, LOCK_SH) != 0) { + close(fd); + return -1; + } + registry_find_ctx_t ctx = {.target = host_pid}; + for_each_record(fd, registry_find_by_host_cb, &ctx); + flock_retry(fd, LOCK_UN); + close(fd); + if (!ctx.found) + return -1; + + /* Guard against host pid reuse: only trust the hit if the pid still + * runs this elfuse binary, same check as proc_get_namespace_targets. + */ + char our_path[PROC_PIDPATHINFO_MAXSIZE]; + int our_len = proc_pidpath(getpid(), our_path, sizeof(our_path)); + if (our_len <= 0) + return -1; + char ppath[PROC_PIDPATHINFO_MAXSIZE]; + int plen = proc_pidpath(host_pid, ppath, sizeof(ppath)); + if (plen != our_len || memcmp(ppath, our_path, (size_t) our_len)) + return -1; + return ctx.guest_pid; +} + int proc_get_child_pids(pid_t *out, int max_pids) { /* Seed with direct children from the process table */ diff --git a/src/syscall/proc.h b/src/syscall/proc.h index cb6e48ad..b57686c7 100644 --- a/src/syscall/proc.h +++ b/src/syscall/proc.h @@ -378,6 +378,14 @@ int proc_set_child_pgid(int64_t guest_pid, int64_t pgid); */ pid_t proc_guest_to_host_pid(int64_t gpid); +/* Look up a host PID in the child process table, falling back to the + * cross-process registry so the rest of the fork family (grandchildren, + * siblings' descendants) resolves too. Returns the guest PID if @host_pid is + * a live member of this guest's fork family, or -1 otherwise (e.g. the lock + * holder is an unrelated host process). + */ +int64_t proc_host_to_guest_pid(pid_t host_pid); + /* Queue a Linux guest signal in a fork-child elfuse process. target_guest_pid * tags the transport record so the receiver drops it if its host pid was * recycled onto a different guest. diff --git a/tests/test-flock.c b/tests/test-flock.c index d22b15d2..bd3b4247 100644 --- a/tests/test-flock.c +++ b/tests/test-flock.c @@ -15,8 +15,10 @@ #include #include +#include #include #include +#include #include #include "test-harness.h" @@ -25,6 +27,7 @@ #define RESERVED_BYTE (PENDING_BYTE + 1) #define SHARED_FIRST (PENDING_BYTE + 2) #define SHARED_SIZE 510 +#define CHILD_LOCK_BYTE 100 static int set_lock(int fd, short type, off_t start, off_t len) { @@ -93,6 +96,53 @@ int main(void) gfl.l_type == F_WRLCK), "F_GETLK returned an invalid l_type"); + /* F_GETLK must report the *guest* PID of a conflicting lock, not + * elfuse's raw host PID -- guest code that treats l_pid as a real PID + * (e.g. a kill(pid, 0) liveness check) would otherwise resolve a foreign + * host process instead of the actual lock holder. + */ + TEST("F_GETLK reports the guest PID of a conflicting child lock"); + int pfd[2]; + int pipe_ok = (pipe(pfd) == 0); + EXPECT_TRUE(pipe_ok, "pipe() failed"); + if (pipe_ok) { + pid_t child = fork(); + if (child == 0) { + close(pfd[0]); + int cfd = open(path, O_RDWR); + if (cfd < 0 || set_lock(cfd, F_WRLCK, CHILD_LOCK_BYTE, 1) != 0) { + _exit(1); + } + pid_t self = getpid(); + ssize_t n = write(pfd[1], &self, sizeof(self)); + if (n != (ssize_t) sizeof(self)) + _exit(1); + sleep(5); /* Hold the lock until the parent has queried it. */ + _exit(0); + } + + close(pfd[1]); + pid_t child_pid = -1; + ssize_t n = read(pfd[0], &child_pid, sizeof(child_pid)); + close(pfd[0]); + + struct flock cfl = { + .l_type = F_WRLCK, + .l_whence = SEEK_SET, + .l_start = CHILD_LOCK_BYTE, + .l_len = 1, + }; + int cr = fcntl(fd, F_GETLK, &cfl); + EXPECT_TRUE(n == (ssize_t) sizeof(child_pid) && cr == 0 && + cfl.l_type == F_WRLCK && cfl.l_pid == child_pid, + "F_GETLK did not report the child's guest PID"); + + if (child > 0) { + kill(child, SIGKILL); + waitpid(child, NULL, 0); + } + } + close(fd); unlink(path); diff --git a/tests/test-matrix.sh b/tests/test-matrix.sh index 0cb35649..e4a937d1 100755 --- a/tests/test-matrix.sh +++ b/tests/test-matrix.sh @@ -626,6 +626,7 @@ run_unit_tests() test_check "$runner" "test-io-opt" "0 failed" "$bindir/test-io-opt" test_check "$runner" "test-poll" "0 failed" "$bindir/test-poll" test_rc "$runner" "test-flock" 0 "$bindir/test-flock" + test_rc "$runner" "test-ofd-lock" 0 "$bindir/test-ofd-lock" test_rc "$runner" "test-times" 0 "$bindir/test-times" test_rc "$runner" "test-syscall-smoke" 0 "$bindir/test-syscall-smoke" test_rc "$runner" "test-vdso" 0 "$bindir/test-vdso" @@ -1207,8 +1208,8 @@ run_suite() # observed counts diverge. apple-unknown is the fallback row for SoC strings the # detector does not recognize yet. EXPECTED_BASELINES=( - "elfuse-aarch64|234|0" - "qemu-aarch64|214|0" + "elfuse-aarch64|235|0" + "qemu-aarch64|215|0" "elfuse-x86_64:apple-m1-m2|71|0" "elfuse-x86_64:apple-m3-plus|71|0" "elfuse-x86_64:apple-unknown|71|0" diff --git a/tests/test-ofd-lock.c b/tests/test-ofd-lock.c new file mode 100644 index 00000000..8840e3d0 --- /dev/null +++ b/tests/test-ofd-lock.c @@ -0,0 +1,137 @@ +/* + * Test open file description (OFD) locking via fcntl(F_OFD_GETLK/SETLK/SETLKW) + * + * Copyright 2026 elfuse contributors + * SPDX-License-Identifier: Apache-2.0 + * + * OFD locks (Linux F_OFD_GETLK=36/F_OFD_SETLK=37/F_OFD_SETLKW=38) are owned + * by the open file description rather than by a process, so Linux always + * reports l_pid=-1 for a conflicting F_OFD_GETLK lock. Passing the host's + * raw l_pid straight through leaks a host PID to the guest and breaks + * software (e.g. SQLite) that checks for the OFD-lock -1 sentinel. + */ + +#include +#include +#include +#include + +#include "test-harness.h" + +static int ofd_lock(int fd, int cmd, short type, off_t start, off_t len) +{ + struct flock fl = { + .l_type = type, + .l_whence = SEEK_SET, + .l_start = start, + .l_len = len, + }; + return fcntl(fd, cmd, &fl); +} + +/* Linux rejects F_OFD_* requests carrying a nonzero l_pid with EINVAL: the + * field is reserved on input since OFD locks are not owned by a process. + */ +static int ofd_lock_with_pid(int fd, int cmd, short type, pid_t pid) +{ + struct flock fl = { + .l_type = type, + .l_whence = SEEK_SET, + .l_start = 0, + .l_len = 16, + .l_pid = pid, + }; + return fcntl(fd, cmd, &fl); +} + +int main(void) +{ + int passes = 0, fails = 0; + const char *path = "/tmp/elfuse-test-ofd-lock.db"; + + int fd1 = open(path, O_RDWR | O_CREAT | O_TRUNC, 0644); + if (fd1 < 0) { + perror("open fd1"); + return 1; + } + /* A second open() yields a distinct open file description, so OFD locks + * taken on fd1 can conflict with a query made through fd2 even though + * both fds live in the same process. + */ + int fd2 = open(path, O_RDWR, 0644); + if (fd2 < 0) { + perror("open fd2"); + return 1; + } + + TEST("F_OFD_SETLK F_WRLCK"); + EXPECT_EQ(ofd_lock(fd1, F_OFD_SETLK, F_WRLCK, 0, 16), 0, + "OFD write lock rejected"); + + TEST("F_OFD_GETLK reports conflict from other OFD"); + struct flock gfl = { + .l_type = F_WRLCK, + .l_whence = SEEK_SET, + .l_start = 0, + .l_len = 16, + }; + int gr = fcntl(fd2, F_OFD_GETLK, &gfl); + EXPECT_TRUE(gr == 0 && gfl.l_type == F_WRLCK, + "F_OFD_GETLK did not report the conflicting write lock"); + + /* This is the behavior issue #129 reports as broken: l_pid must be -1 + * for an OFD lock conflict, never a real (host or guest) PID. + */ + TEST("F_OFD_GETLK l_pid is -1 on conflict"); + EXPECT_EQ(gfl.l_pid, -1, "F_OFD_GETLK leaked a non -1 l_pid"); + + TEST("F_OFD_SETLK F_UNLCK"); + EXPECT_EQ(ofd_lock(fd1, F_OFD_SETLK, F_UNLCK, 0, 16), 0, + "OFD unlock rejected"); + + TEST("F_OFD_GETLK reports no conflict after unlock"); + struct flock gfl2 = { + .l_type = F_WRLCK, + .l_whence = SEEK_SET, + .l_start = 0, + .l_len = 16, + }; + int gr2 = fcntl(fd2, F_OFD_GETLK, &gfl2); + EXPECT_TRUE(gr2 == 0 && gfl2.l_type == F_UNLCK, + "F_OFD_GETLK still reported a lock after unlock"); + + /* Blocking variant must take the same translation path. */ + TEST("F_OFD_SETLKW F_WRLCK"); + EXPECT_EQ(ofd_lock(fd2, F_OFD_SETLKW, F_WRLCK, 0, 16), 0, + "F_OFD_SETLKW rejected"); + EXPECT_EQ(ofd_lock(fd2, F_OFD_SETLK, F_UNLCK, 0, 16), 0, + "OFD unlock (fd2) rejected"); + + /* A nonzero l_pid in the request is reserved/invalid for all three OFD + * commands, matching fs/locks.c fcntl_getlk/fcntl_setlk. + */ + TEST("F_OFD_SETLK rejects nonzero l_pid"); + errno = 0; + EXPECT_EQ(ofd_lock_with_pid(fd1, F_OFD_SETLK, F_WRLCK, getpid()), -1, + "F_OFD_SETLK accepted a nonzero l_pid"); + EXPECT_EQ(errno, EINVAL, "F_OFD_SETLK with nonzero l_pid: wrong errno"); + + TEST("F_OFD_SETLKW rejects nonzero l_pid"); + errno = 0; + EXPECT_EQ(ofd_lock_with_pid(fd1, F_OFD_SETLKW, F_WRLCK, getpid()), -1, + "F_OFD_SETLKW accepted a nonzero l_pid"); + EXPECT_EQ(errno, EINVAL, "F_OFD_SETLKW with nonzero l_pid: wrong errno"); + + TEST("F_OFD_GETLK rejects nonzero l_pid"); + errno = 0; + EXPECT_EQ(ofd_lock_with_pid(fd1, F_OFD_GETLK, F_WRLCK, getpid()), -1, + "F_OFD_GETLK accepted a nonzero l_pid"); + EXPECT_EQ(errno, EINVAL, "F_OFD_GETLK with nonzero l_pid: wrong errno"); + + close(fd1); + close(fd2); + unlink(path); + + SUMMARY("test-ofd-lock"); + return fails == 0 ? 0 : 1; +}