Skip to content

Let kill reach a relative that is not a child #388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xalestar wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Let kill reach a relative that is not a child #388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2555e69
Let kill reach a relative that is not a child
xalestar Sep 13, 2026
e0012bb
Tie a registry record to its process start time
xalestar Sep 18, 2026
464b77a
Fail the stale-registry lane on setup errors
xalestar Sep 18, 2026
2cc9ea2
Fail the lane if the stale-registry holder died
xalestar Sep 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion mk/tests.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ ELFUSE_HOST_NOFILE_MIN ?= $(shell bash "$(CURDIR)/tests/test-config.sh" --host-n
test-sysroot-dotdot test-sysroot-openat2-walk \
test-sysroot-inotify-names test-sysroot-exec-names \
test-sysroot-interp-fallback test-sysroot-interp-cased \
test-sysroot-absock-names test-absock-cleanup \
test-sysroot-absock-names test-absock-cleanup test-registry-stale-pid \
test-linkat-symlink-fallback test-casefold-host \
test-casefold-walk-host test-absock-names-host \
test-wakeup-pipe-host test-guest-env-host \
Expand Down Expand Up @@ -340,6 +340,7 @@ check: $(ELFUSE_BIN) $(TEST_DEPS) check-syscall-coverage check-eintr-contract ch
$(call run-lane,test-sysroot-interp-cased,PT_INTERP through an escaped path)
$(call run-lane,test-sysroot-absock-names,pathname sockets across the escape boundary)
$(call run-lane,test-absock-cleanup,absock namespace lifecycle)
$(call run-lane,test-registry-stale-pid,stale registry record on a reused host pid)
$(call run-lane,test-sysroot-root,sysroot mounted at /)
$(call run-lane,test-nosysroot-literal-names,literal names without a sysroot)
$(call run-lane,test-sysroot-outside-names,literal names outside the sysroot)
Expand Down Expand Up @@ -726,6 +727,39 @@ test-absock-cleanup: $(ELFUSE_BIN) $(BUILD_DIR)/test-absock-cleanup
fi; \
$(ASSERT_NO_ABSOCK_LEAK)

# An exited member's registry record outlives it, and macOS can hand its host
# pid to another elfuse process. The recipe plants such a record, host pid of
# a live unrelated elfuse run with a start time it does not have, and the
# family's kill(99, 0) must still fail with ESRCH.
## registry ignores a record whose host pid was reused
test-registry-stale-pid: $(ELFUSE_BIN) $(BUILD_DIR)/test-registry-stale-pid
@tmp=$$(mktemp -d); xpid=; fpid=; \
trap 'kill $$xpid $$fpid 2>/dev/null; rm -rf "$$tmp"' EXIT; \
fail() { printf "FAIL: %s\n" "$$1"; exit 1; }; \
printf " %-30s " "stale record on reused pid"; \
mkfifo "$$tmp/go"; \
$(ELFUSE_BIN) $(BUILD_DIR)/test-registry-stale-pid hold & \
xpid=$$!; \
$(ELFUSE_BIN) $(BUILD_DIR)/test-registry-stale-pid \
< "$$tmp/go" > "$$tmp/out" & \
fpid=$$!; \
exec 4> "$$tmp/go"; \
for i in $$(seq 1 50); do \
grep -q READY "$$tmp/out" && break; \
sleep 0.1; \
done; \
grep -q READY "$$tmp/out" || fail "family never reported READY"; \
reg="$$(getconf DARWIN_USER_TEMP_DIR)elfuse-procs-$$fpid"; \
[ -f "$$reg" ] || fail "no registry at $$reg"; \
printf '%s 99 1 1\n' "$$xpid" >> "$$reg" || fail "cannot append to $$reg"; \
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
echo go >&4; \
exec 4>&-; \
wait $$fpid; \

@jserv jserv Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait $$fpid has no bound, so a regression that leaves the guest parked (the read(0, ...) on the fifo, or the forked child's read(release[0])) hangs this recipe instead of failing it. The EXIT trap never runs, so make check stops here with both elfuse roots still alive and the scratch directory left behind. Every other lane in this file reaches the guest through a runner that wraps the invocation in timeout; this recipe launches $(ELFUSE_BIN) directly. Wrapping both launches in timeout $(TEST_TIMEOUT) and failing when fpid outlives it keeps a hang reportable.

kill -0 "$$xpid" 2>/dev/null || fail "holder exited before the lookup"; \
verdict=$$(sed -n 's/^STALE=//p' "$$tmp/out"); \
[ "$$verdict" = esrch ] || fail "kill(99, 0) $${verdict:-unreported}"; \
printf "OK\n"

# PT_INTERP names the loader by the guest's spelling, and a rootfs may ship
# it somewhere other than where the binary asks (store-style paths). The
# interp resolver falls back to /lib/<basename> when the asked-for path does
Expand Down
79 changes: 63 additions & 16 deletions src/syscall/proc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1157,11 +1157,14 @@ static void proc_registry_reset_if_owner(const char *path)
unlink(path);
}

/* One live member of a process group registry. */
/* One live member of a process group registry. start_us is the host process's
* start time: a host pid alone matches whatever process macOS hands it to next.
*/
typedef struct {
pid_t host_pid;
int64_t guest_pid;
int64_t pgid;
uint64_t start_us;
} registry_entry_t;

#define REGISTRY_MAX_ENTRIES 4096
Expand All @@ -1178,10 +1181,10 @@ static int flock_retry(int fd, int op)

/* Read @fd from its current offset and invoke @cb once per newline-terminated
* record, passing a NUL-terminated copy. Records must fit in 159 bytes; both
* the registry ("hostpid guestpid pgid") and signal/control transport records
* use bounded numeric lines. Overlong records and an unterminated trailing
* token are dropped -- every writer appends a whole record under an exclusive
* lock, so a partial line only appears after a crash mid-write.
* the registry ("hostpid guestpid pgid startus") and signal/control transport
* records use bounded numeric lines. Overlong records and an unterminated
* trailing token are dropped -- every writer appends a whole record under an
* exclusive lock, so a partial line only appears after a crash mid-write.
*/
static void for_each_record(int fd, void (*cb)(char *rec, void *ctx), void *ctx)
{
Expand Down Expand Up @@ -1217,19 +1220,37 @@ typedef struct {
bool truncated;
} registry_parse_ctx_t;

/* Upsert one "hostpid guestpid pgid" record, keeping the latest guest_pid/pgid
* per LIVE host pid. Dead, malformed, and out-of-range records are dropped.
/* Start time of host process @pid in microseconds.
*
* Returns false once @pid has exited.
*/
static bool host_start_us(pid_t pid, uint64_t *out)

@jserv jserv Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing on the reverse lookup uses this. registry_find_by_host_cb still parses three fields, and proc_host_to_guest_pid still accepts a hit on host pid plus a matching proc_pidpath, which is the exact identity test this commit demonstrates is not sufficient. Once macOS hands an exited member's host pid to another elfuse process, F_GETLK reports the exited member's guest pid as the conflicting lock holder, the same stale-record class the second commit closes for kill. Parsing the fourth field in registry_find_by_host_cb and comparing it against host_start_us closes it on that path too.

{
struct proc_bsdinfo info;
if (proc_pidinfo(pid, PROC_PIDTBSDINFO, 0, &info, sizeof(info)) !=
(int) sizeof(info))
return false;
*out = info.pbi_start_tvsec * 1000000ULL + info.pbi_start_tvusec;
return true;
}

/* Upsert one "hostpid guestpid pgid startus" record, keeping the latest
* guest_pid/pgid per LIVE host pid. A record whose start time differs from the
* running process's names an exited member whose host pid was reused, so it is
* dropped along with dead, malformed, and out-of-range records.
*/
static void registry_parse_cb(char *rec, void *vctx)
{
registry_parse_ctx_t *c = vctx;
long hp;
long long gp, pg;
if (sscanf(rec, "%ld %lld %lld", &hp, &gp, &pg) != 3)
unsigned long long st;
if (sscanf(rec, "%ld %lld %lld %llu", &hp, &gp, &pg, &st) != 4)
return;
if (hp <= 0 || hp > INT_MAX || pg < 0 || pg > INT_MAX)
return;
if (kill((pid_t) hp, 0) != 0)
uint64_t live_us;
if (!host_start_us((pid_t) hp, &live_us) || live_us != (uint64_t) st)
return;
int idx = -1;
for (int k = 0; k < c->n; k++)
Expand All @@ -1244,6 +1265,7 @@ static void registry_parse_cb(char *rec, void *vctx)
}
idx = c->n++;
c->entries[idx].host_pid = (pid_t) hp;
c->entries[idx].start_us = live_us;
}
c->entries[idx].guest_pid = (int64_t) gp;
c->entries[idx].pgid = (int64_t) pg;
Expand Down Expand Up @@ -1328,7 +1350,8 @@ static void proc_registry_publish(pid_t host_pid,
idx = i;
break;
}
if (idx < 0) {
uint64_t start_us;
if (idx < 0 && host_start_us(host_pid, &start_us)) {
if (n == REGISTRY_MAX_ENTRIES)

/* No slot for a new live member: group signals (kill(-1),
Expand All @@ -1342,6 +1365,7 @@ static void proc_registry_publish(pid_t host_pid,
else {
idx = n++;
entries[idx].host_pid = host_pid;
entries[idx].start_us = start_us;
}
}
if (idx >= 0) {
Expand All @@ -1351,11 +1375,12 @@ static void proc_registry_publish(pid_t host_pid,

if (ftruncate(fd, 0) == 0 && lseek(fd, 0, SEEK_SET) == 0) {
for (int i = 0; i < n; i++) {
char lineb[64];
int len = snprintf(lineb, sizeof(lineb), "%ld %lld %lld\n",
char lineb[96];
int len = snprintf(lineb, sizeof(lineb), "%ld %lld %lld %llu\n",
(long) entries[i].host_pid,
(long long) entries[i].guest_pid,
(long long) entries[i].pgid);
(long long) entries[i].pgid,
(unsigned long long) entries[i].start_us);
if (len > 0 && (size_t) len < sizeof(lineb) &&
write_all(fd, lineb, (size_t) len) < 0)
break;
Expand Down Expand Up @@ -1708,9 +1733,14 @@ int proc_set_child_pgid(int64_t guest_pid_val, int64_t pgid)
return ret;
}

int proc_get_namespace_targets(proc_signal_target_t *out,
int max,
int64_t pgid_filter)
/* Shared body for the group/broadcast collector and the single-pid lookup.
* guest_filter of 0 accepts every member; a positive value stops at the one
* member carrying that guest pid.
*/
static int registry_collect(proc_signal_target_t *out,
int max,
int64_t pgid_filter,
int64_t guest_filter)
{
/* No republish here: every group change already publishes (fork, setpgid,
* setsid), and this reader excludes its own entry anyway.
Expand Down Expand Up @@ -1749,6 +1779,8 @@ int proc_get_namespace_targets(proc_signal_target_t *out,
continue;
if (pgid_filter != PROC_PGID_ANY && entries[i].pgid != pgid_filter)
continue;
if (guest_filter > 0 && entries[i].guest_pid != guest_filter)
continue;
char ppath[PROC_PIDPATHINFO_MAXSIZE];
int plen = proc_pidpath(entries[i].host_pid, ppath, sizeof(ppath));
if (plen != our_len || memcmp(ppath, our_path, (size_t) our_len))
Expand All @@ -1760,6 +1792,21 @@ int proc_get_namespace_targets(proc_signal_target_t *out,
return count;
}

int proc_get_namespace_targets(proc_signal_target_t *out,
int max,
int64_t pgid_filter)
{
return registry_collect(out, max, pgid_filter, 0);
}

pid_t proc_namespace_host_pid(int64_t guest_pid)
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
{
proc_signal_target_t target;
Comment on lines +1802 to +1804

@jserv jserv Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two inputs break the contract proc.h states for this function. A guest_pid of 0 or negative leaves guest_filter > 0 false inside registry_collect, which disables the filter entirely and returns the first non-self member, so the caller gets an arbitrary host pid where the header promises -1. The caller's own guest pid returns -1 even though it is a live member, because registry_collect skips host_pid == self. sc_kill screens both cases before it calls, so nothing is wrong today, but this is exported as a general resolver and the next caller has no reason to know it must screen.

Suggested change
pid_t proc_namespace_host_pid(int64_t guest_pid)
{
proc_signal_target_t target;
pid_t proc_namespace_host_pid(int64_t guest_pid)
{
if (guest_pid <= 0)
return -1;
proc_signal_target_t target;

return registry_collect(&target, 1, PROC_PGID_ANY, guest_pid) > 0
? target.host_pid
: -1;
}

int64_t proc_host_to_guest_pid(pid_t host_pid)
{
pthread_mutex_lock(&pid_lock);
Expand Down
10 changes: 10 additions & 0 deletions src/syscall/proc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,16 @@ int proc_get_namespace_targets(proc_signal_target_t *out,
int max,
int64_t pgid_filter);

/* Resolve one guest pid to its host pid through the same fork-family registry
* proc_get_namespace_targets reads. The child table only holds descendants, so
* this is what lets a process signal a relative that is not its own child (its
* parent, most commonly).
*
* Returns the host pid, or -1 when the registry holds no live member with that
* guest pid.
*/
pid_t proc_namespace_host_pid(int64_t guest_pid);

@jserv jserv Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sys_pidfd_open and sys_pidfd_send_signal still resolve through proc_guest_to_host_pid alone, so after this change kill(getppid(), 0) succeeds while pidfd_open(getppid()) still returns ESRCH, and a pidfd on any relative that is not a descendant stays unreachable. Linux draws no such line. Worth a follow-up routing the three proc_guest_to_host_pid call sites in src/syscall/proc-pidfd.c through the same resolver.


/* Publish the caller's current guest pid/pgid to the fork-family registry. */
void proc_registry_publish_self(void);

Expand Down
15 changes: 13 additions & 2 deletions src/syscall/syscall.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,6 +1034,17 @@ static int kill_deliver_targets(const proc_signal_target_t *targets,
return delivered;
}

/* Resolve a guest pid for kill(2). The child table answers for descendants.
* Every other member of the fork family, the caller's own parent above all,
* exists only in the namespace registry, the same source the group and
* broadcast forms already read.
*/
static pid_t kill_resolve_host_pid(int64_t gpid)

@jserv jserv Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This puts pid-directed kill on the registry, where before only the group and broadcast forms read it, and the registry's file name is keyed on absock_get_namespace_id(), which falls back to the root's host pid. macOS recycles that pid like any other: once a family's root exits with a child still running, a new elfuse root that lands on the reused pid passes the owner test in proc_registry_reset_if_owner, unlinks the orphan family's registry, and then publishes into the same path. The start time added by the second commit proves the target is live and proc_pidpath proves it runs the same binary, but neither proves it is in the same family, so the orphan's kill(1, SIGKILL) reaches the unrelated root, and proc_send_guest_signal passes it through because the namespace tag it checks is that same colliding value. Keying the registry on something that is not a recycled pid, a nonce carried through the existing fork IPC state for instance, would close it for the group forms as well.

{
pid_t hpid = proc_guest_to_host_pid(gpid);
return hpid > 0 ? hpid : proc_namespace_host_pid(gpid);
}

static int64_t sc_kill(guest_t *g,
uint64_t x0,
uint64_t x1,
Expand Down Expand Up @@ -1083,7 +1094,7 @@ static int64_t sc_kill(guest_t *g,
}
int64_t r = (pid == (int) our_pid) ? 0 : -LINUX_ESRCH;
if (r == -LINUX_ESRCH) {
pid_t hpid = proc_guest_to_host_pid((int64_t) pid);
pid_t hpid = kill_resolve_host_pid((int64_t) pid);
if (hpid > 0)
r = (kill(hpid, 0) == 0) ? 0 : -LINUX_ESRCH;
}
Expand Down Expand Up @@ -1148,7 +1159,7 @@ static int64_t sc_kill(guest_t *g,
signal_queue(sig);
return 0;
}
pid_t hpid = proc_guest_to_host_pid((int64_t) pid);
pid_t hpid = kill_resolve_host_pid((int64_t) pid);
if (hpid > 0)
return (proc_send_guest_signal(hpid, (int64_t) pid, sig) == 0)
? 0
Expand Down
74 changes: 74 additions & 0 deletions tests/test-kill-parent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Test kill(pid, sig) aimed at the caller's parent
*
* Copyright 2026 elfuse contributors
* SPDX-License-Identifier: Apache-2.0
*
* A forked child probes its parent with kill(getppid(), 0) and then signals it
* with SIGUSR1. The parent is not in the child's own descendant table, so both
* calls exercise the single-pid lookup through the fork-family registry.
*/

#include <signal.h>
#include <stdbool.h>
#include <stdio.h>
#include <string.h>
#include <sys/wait.h>
#include <time.h>
#include <unistd.h>

static volatile sig_atomic_t got_usr1 = 0;

static void usr1_handler(int sig)
{
(void) sig;
got_usr1 = 1;
}

static bool wait_flag(int max_ms)
{
for (int i = 0; i < max_ms && !got_usr1; i++) {
struct timespec ts = {0, 1000000}; /* 1 ms */
nanosleep(&ts, NULL);
}
return got_usr1 != 0;
}

int main(void)
{
int failed = 0;
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sa.sa_handler = usr1_handler;
sigaction(SIGUSR1, &sa, NULL);

pid_t pid = fork();
if (pid < 0)
return 1;
if (pid == 0) {
if (kill(getppid(), 0) != 0)
_exit(1);
if (kill(getppid(), SIGUSR1) != 0)
_exit(2);
_exit(0);
}

if (!wait_flag(2000)) {
fprintf(stderr, "FAIL: child kill(getppid(), SIGUSR1) not delivered\n");
failed++;
}
int status = 0;
if (waitpid(pid, &status, 0) != pid || !WIFEXITED(status)) {
fprintf(stderr, "FAIL: child did not exit cleanly\n");
failed++;
} else if (WEXITSTATUS(status) == 1) {
fprintf(stderr, "FAIL: kill(getppid(), 0) failed in child\n");
failed++;
} else if (WEXITSTATUS(status) == 2) {
fprintf(stderr, "FAIL: kill(getppid(), SIGUSR1) failed in child\n");
failed++;
}

printf("%s: %d failed\n", failed == 0 ? "PASS" : "FAIL", failed);
return failed == 0 ? 0 : 1;
}
2 changes: 2 additions & 0 deletions tests/test-matrix.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,8 @@ run_unit_tests()
"$bindir/test-kill-broadcast"
test_check "$runner" "test-kill-pgroup" "0 failed" \
"$bindir/test-kill-pgroup"
test_check "$runner" "test-kill-parent" "0 failed" \
"$bindir/test-kill-parent"
test_rc "$runner" "test-sigio" 0 "$bindir/test-sigio"
test_rc "$runner" "test-fault-signal-mt" 0 "$bindir/test-fault-signal-mt"
test_rc "$runner" "test-exit-group-worker" 0 "$bindir/test-exit-group-worker"
Expand Down
Loading
Loading