From 26de80c6a4dbe712f848fc2ec3fd696d0a265f79 Mon Sep 17 00:00:00 2001 From: Trung Date: Fri, 7 Aug 2026 16:13:39 +0700 Subject: [PATCH] Report a pty hangup on every path that can observe one epoll_wait never reported EPOLLHUP for a pty master whose slaves had all closed, and slaves obtained through TIOCGPTPEER were never registered, so the master's hangup accounting missed them. Wire both, and re-verify the fd generation a registration pinned at ADD/MOD so a close-and-reuse in the lookup window cannot charge the hangup to an unrelated file. Drop proc_pty_any_master_hung_up(). It was meant as a cheap gate that let epoll skip the registration sweep, but it latches: a master that hangs up while the guest still holds its fd keeps the gate true for the process's lifetime, and every epoll_pwait then paid for a full FD_TABLE_SIZE sweep. Track per-instance active and pty-master counts instead, so the scan is bounded by what the instance actually registered and skipped entirely when it holds no pty master. Delete kqueue events whose guest fd is no longer registered. A level-triggered event for a deregistered fd is re-reported on every call and translates to zero epoll events, which spins the caller. Keep an epoll registration alive while another fd still refers to the same open file description, matching Linux, and identify that description by ofd_id at the close chokepoint. Return ready events in preference to EINTR. Linux ep_poll() checks ep_events_available() and jumps to send_events before it consults signal_pending, so EINTR is only the answer for a wait that produced nothing. Discarding a ready fd to report EINTR loses it in practice: kqueue re-reports it, but the same pending signal is still there, so the caller is handed EINTR forever and never drains the fd. A terminal with an unhandled SIGCHLD spun at 100% CPU this way without ever drawing. Fix #274 --- src/main.c | 11 +- src/runtime/forkipc.c | 52 ++++ src/runtime/procemu.c | 577 ++++++++++++++++++++++++++++++++++++++--- src/runtime/procemu.h | 57 +++- src/syscall/fdtable.c | 16 +- src/syscall/fs.c | 11 +- src/syscall/internal.h | 60 +++++ src/syscall/io.c | 91 ++++++- src/syscall/poll.c | 297 ++++++++++++++++----- src/syscall/poll.h | 2 +- tests/test-pty.c | 379 +++++++++++++++++++++++++++ 11 files changed, 1441 insertions(+), 112 deletions(-) diff --git a/src/main.c b/src/main.c index 6c410c1f..36d79a2c 100644 --- a/src/main.c +++ b/src/main.c @@ -36,7 +36,8 @@ #include "core/sysroot.h" #include "runtime/forkipc.h" -#include "runtime/futex.h" /* futex_interrupt_request */ +#include "runtime/futex.h" /* futex_interrupt_request */ +#include "runtime/procemu.h" /* proc_pty_release_process_slaves */ #include "runtime/proctitle.h" #include "runtime/thread.h" @@ -762,6 +763,14 @@ int main(int argc, char **argv) * temp ELF, which the post-prepare error paths and a Rosetta guest's * teardown previously leaked. */ + /* Give back any pty slaves this process still holds before the fd table + * goes away. The guest's stdio slaves are closed by the kernel, not by the + * guest, so they never pass through the per-fd close hook; a master in + * another process would otherwise wait forever for a hangup that this exit + * should have produced. + */ + proc_pty_release_process_slaves(); + cleanup_main_resources(&g, guest_initialized, &sysroot_mount, have_host_cwd ? host_cwd : NULL, guest_argv, guest_argc, elf_path, sysroot_path); diff --git a/src/runtime/forkipc.c b/src/runtime/forkipc.c index 76a9ea32..b2fcd6bb 100644 --- a/src/runtime/forkipc.c +++ b/src/runtime/forkipc.c @@ -36,6 +36,7 @@ #include "runtime/forkipc.h" #include "runtime/fork-state.h" #include "runtime/futex.h" +#include "runtime/procemu.h" #include "syscall/linux-wire.h" #include "syscall/chown-overlay.h" @@ -294,15 +295,34 @@ int fork_child_main(int ipc_fd, return 1; } + /* Both the fd table and the keepalives are in place, which is what this + * needs to recognize the slave fds inherited from the parent and put them + * back on the books. Without it the parent's close of its own copy makes + * the pty look hung up while this child still holds a live slave. + */ + proc_pty_adopt_inherited_slaves(); + signal_state_t sig; if (fork_ipc_recv_process_state(ipc_fd, &g, &sig) < 0) { log_error("fork-child: failed to receive process state"); + /* Give back the credit the parent added for this child before + * bailing: nothing here reaches the teardown that normally + * returns it, and a child that never runs must not leave the + * pty looking busy to the master the parent still holds. + */ + proc_pty_release_process_slaves(); guest_destroy(&g); return 1; } if (chown_overlay_recv(ipc_fd) < 0) { log_error("fork-child: failed to receive chown overlay"); + /* Give back the credit the parent added for this child before + * bailing: nothing here reaches the teardown that normally + * returns it, and a child that never runs must not leave the + * pty looking busy to the master the parent still holds. + */ + proc_pty_release_process_slaves(); guest_destroy(&g); return 1; } @@ -317,6 +337,12 @@ int fork_child_main(int ipc_fd, 0 || admission_ready != 1) { log_error("fork-child: parent did not commit child admission"); + /* Give back the credit the parent added for this child before + * bailing: nothing here reaches the teardown that normally + * returns it, and a child that never runs must not leave the + * pty looking busy to the master the parent still holds. + */ + proc_pty_release_process_slaves(); guest_destroy(&g); return 1; } @@ -371,6 +397,12 @@ int fork_child_main(int ipc_fd, * the single-threaded child at this point). */ if (shim_globals_install_per_vcpu(vcpu, &g, hdr.child_pid) < 0) { + /* Give back the credit the parent added for this child before + * bailing: nothing here reaches the teardown that normally + * returns it, and a child that never runs must not leave the + * pty looking busy to the master the parent still holds. + */ + proc_pty_release_process_slaves(); guest_destroy(&g); return 1; } @@ -498,6 +530,14 @@ int fork_child_main(int ipc_fd, vcpu_run_loop(vcpu, vexit, &g, verbose, timeout_sec, &wait_status); proc_process_exit(wait_status); + + /* Same reason as the main-process path: this child is where a forked shell + * runs, and its stdio slaves are closed by the kernel rather than by the + * guest, so they never reach the per-fd close hook. The parent still holds + * the master and is waiting for exactly this hangup. + */ + proc_pty_release_process_slaves(); + guest_destroy(&g); return exit_code; } @@ -1911,6 +1951,18 @@ int64_t sys_clone(hv_vcpu_t vcpu, goto fail_snapshot; } + /* The child's inherited slave copies are live from the moment fork returns, + * so they go on the shared count here rather than in the child's own init, + * which the parent's close of its copy can beat. + * + * Deliberately after the last failure exit: every goto above unwinds a + * child that will never run, and a credit added before them would never be + * given back -- the pty would then look permanently busy and its master + * would stop reporting hangups for good. Still before siblings resume, so + * no guest code can close a slave in between. + */ + proc_pty_fork_parent_note_inherited(); + /* The process-state payload includes the SCM_RIGHTS handoff for region * backing fds. Keep siblings quiesced until that send completes so a * concurrent munmap/remap cannot close or recycle the captured fd numbers. diff --git a/src/runtime/procemu.c b/src/runtime/procemu.c index 510600e7..9418959c 100644 --- a/src/runtime/procemu.c +++ b/src/runtime/procemu.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -1590,6 +1591,9 @@ static void proc_task_collect_cb(thread_entry_t *t, void *arg) * local master closes clear the mapping immediately. */ #define PTY_KEEPALIVE_MAX 256 + +/* macOS caps shm names (PSHMNAMLEN) at 31 bytes including the leading slash. */ +#define PTY_SHM_NAME_MAX 32 #define PTY_KEEPALIVE_FREE (-1) /* Group that owns pty slaves. Linux distributions mount devpts with gid=5 @@ -1637,6 +1641,30 @@ static bool pty_slave_num_from_path(const char *path, uint32_t *out) /* PTY_SLAVE_PATH_MAX lives in procemu.h so this table and the fork-IPC payload * (proc_pty_ipc_entry_t) cannot drift apart. */ +/* Cross-process slave accounting for one pty. + * + * The per-process counters below cannot answer the hangup question on their + * own, because a guest fork is a posix_spawn of a fresh elfuse process (see + * forkipc.c): the child gets its own keepalive table, so the slave a shell + * opens after the fork is invisible to the parent that owns the master and + * polls it. That is the whole terminal case -- foot holds the master and never + * opens a slave in that process -- so the master would never report a hangup. + * + * Anonymous shared memory cannot cross posix_spawn either, so this lives in a + * shm segment named after the host slave path, which is unique per host pty and + * which both sides already know: the parent from its own open, the child from + * the slave_path in the fork-IPC keepalive payload. No new IPC is needed. + * + * Counters are atomic rather than mutex-guarded on purpose: a process that dies + * holding a process-shared mutex would wedge every other process on this pty, + * and macOS has no robust mutexes. + */ +typedef struct { + _Atomic int32_t refs; /* elfuse processes holding a keepalive */ + _Atomic int32_t slave_count; /* guest-held slaves across all of them */ + _Atomic int32_t seen; /* a guest slave existed at least once */ +} pty_shared_t; + static struct { int master_host_fd; int slave_host_fd; @@ -1645,9 +1673,17 @@ static struct { /* Slaves the guest has open, and whether it ever had one. Both are needed: * a count of zero means hung up only after the first open. + * + * These stay per-process and are the fallback when the shared segment is + * unavailable (shm_open denied, for instance), which degrades to the + * same-process-only behavior rather than failing. guest_slave_count doubles + * as this process's contribution to shared->slave_count, so detaching can + * subtract it and stay balanced even when the guest exits without running + * per-fd cleanup. */ int guest_slave_count; bool guest_slave_seen; + pty_shared_t *shared; char slave_path[PTY_SLAVE_PATH_MAX]; } pty_keepalive_table[PTY_KEEPALIVE_MAX]; @@ -1673,6 +1709,156 @@ static struct { static pthread_mutex_t pty_keepalive_lock = PTHREAD_MUTEX_INITIALIZER; static pthread_once_t pty_keepalive_once = PTHREAD_ONCE_INIT; +/* Derive the shm name for a pty from its host slave path. The basename is + * unique per host pty ("ttys004"), which is what makes the segment findable + * from a spawned child holding nothing but the path. macOS caps shm names at 31 + * bytes including the leading slash, so the prefix is kept short. + */ +/* Pty accounting trace. + * + * Writes to the file named by ELFUSE_PTY_LOG when set, in addition to the + * normal DEBUG log. The file matters because this subsystem spans processes: + * the parent holding the master and the child holding the slave are different + * elfuse instances, launched by a GUI app whose stderr goes nowhere reachable, + * so a shared append-only file with a pid tag is the only way to see both + * halves of a hangup decision in one place. + */ +__attribute__((format(printf, 1, 2))) static void pty_diag(const char *fmt, ...) +{ + static _Atomic int diag_fd = -2; /* -2 unopened, -1 disabled */ + int fd = atomic_load(&diag_fd); + if (fd == -2) { + const char *path = getenv("ELFUSE_PTY_LOG"); + int opened = -1; + if (path && path[0]) + opened = + open(path, O_WRONLY | O_CREAT | O_APPEND | O_CLOEXEC, 0644); + int expected = -2; + if (!atomic_compare_exchange_strong(&diag_fd, &expected, opened)) { + if (opened >= 0) + close(opened); + fd = atomic_load(&diag_fd); + } else { + fd = opened; + } + } + if (fd < 0) + return; + + char msg[512]; + va_list ap; + va_start(ap, fmt); + int n = vsnprintf(msg, sizeof(msg), fmt, ap); + va_end(ap); + if (n < 0) + return; + + char line[600]; + int m = snprintf(line, sizeof(line), "[pid %d] %s\n", (int) getpid(), msg); + if (m > 0) + (void) !write(fd, line, (size_t) m); +} + +static bool pty_shared_name(const char *slave_path, char *out, size_t out_sz) +{ + if (!slave_path || slave_path[0] == '\0') + return false; + const char *base = strrchr(slave_path, '/'); + base = base ? base + 1 : slave_path; + if (base[0] == '\0') + return false; + /* Reject anything that is not a plain name so the path cannot inject a + * separator into the shm namespace. + */ + for (const char *p = base; *p; p++) { + if (!isalnum((unsigned char) *p) && *p != '_' && *p != '-') + return false; + } + int n = snprintf(out, out_sz, "/elfuse.pty.%s", base); + return n > 0 && (size_t) n < out_sz; +} + +/* Map this pty's shared counters, creating the segment when absent. + * + * fresh discards any segment left behind by a previous master on the same host + * pty: the path is only recycled once the host tty is fully released, so a + * surviving segment is stale state from a process that died without detaching. + * The fork-restore path passes false, since joining the parent's live segment + * is the entire point there. + * + * Returns NULL when the segment is unavailable; callers fall back to the + * per-process counters. + */ +static pty_shared_t *pty_shared_attach(const char *slave_path, bool fresh) +{ + char name[PTY_SHM_NAME_MAX]; + if (!pty_shared_name(slave_path, name, sizeof(name))) + return NULL; + if (fresh) + shm_unlink(name); + + bool created = true; + int fd = shm_open(name, O_RDWR | O_CREAT | O_EXCL, 0600); + if (fd < 0 && errno == EEXIST) { + created = false; + fd = shm_open(name, O_RDWR, 0600); + } + if (fd < 0) + return NULL; + + if (created && ftruncate(fd, sizeof(pty_shared_t)) < 0) { + close(fd); + shm_unlink(name); + return NULL; + } + if (!created) { + /* The creator sizes the segment just after shm_open, so a joiner that + * lands in that gap would map a zero-length object and take SIGBUS on + * first touch. Bail to the per-process fallback instead of waiting: + * this runs under fd_lock (proc_pty_master_adopt registers with both + * pty_keepalive_lock and fd_lock held), where sleeping would stall + * every fd operation in the process. + */ + struct stat st; + if (fstat(fd, &st) != 0 || st.st_size < (off_t) sizeof(pty_shared_t)) { + close(fd); + return NULL; + } + } + + void *map = mmap(NULL, sizeof(pty_shared_t), PROT_READ | PROT_WRITE, + MAP_SHARED, fd, 0); + close(fd); + if (map == MAP_FAILED) + return NULL; + + pty_shared_t *sh = map; + atomic_fetch_add(&sh->refs, 1); + return sh; +} + +/* Drop this process's reference, handing back any slaves it still had counted, + * and unlink the segment once the last process lets go. + */ +static void pty_shared_detach(pty_shared_t *sh, + const char *slave_path, + int local_slave_count) +{ + if (!sh) + return; + if (local_slave_count > 0) { + pty_diag("pty: detach returns %d slave(s) path=%s", local_slave_count, + slave_path ? slave_path : "?"); + atomic_fetch_sub(&sh->slave_count, local_slave_count); + } + if (atomic_fetch_sub(&sh->refs, 1) == 1) { + char name[PTY_SHM_NAME_MAX]; + if (pty_shared_name(slave_path, name, sizeof(name))) + shm_unlink(name); + } + munmap(sh, sizeof(*sh)); +} + /* Sentinel-init. Other fields stay BSS-zero; without sentinels a host fd 0 * close would match slot 0 and close the wrong fd inside elfuse. */ @@ -1704,6 +1890,10 @@ static int pty_keepalive_find_master_locked(int master_host_fd) static int pty_keepalive_clear_slot_locked(int slot) { int slave = pty_keepalive_table[slot].slave_host_fd; + pty_shared_detach(pty_keepalive_table[slot].shared, + pty_keepalive_table[slot].slave_path, + pty_keepalive_table[slot].guest_slave_count); + pty_keepalive_table[slot].shared = NULL; pty_keepalive_table[slot].master_host_fd = PTY_KEEPALIVE_FREE; pty_keepalive_table[slot].guest_slave_count = 0; pty_keepalive_table[slot].guest_slave_seen = false; @@ -1714,6 +1904,27 @@ static int pty_keepalive_clear_slot_locked(int slot) return slave; } +/* Consume a stale entry's one-shot open without giving up its accounting. + * + * pty_open_slave retires the entry as soon as it has translated the + * close-before-open sequence, but the caller only records the guest slave + * afterwards. Clearing the slot outright detached the shared segment first, so + * that slave was credited to nobody and the master -- still held by the parent + * -- never learned the shell had one. What has to be consumed is the one-shot + * marker and the retained fd; the path, pts number and shared mapping stay so + * the slot remains the pty's accounting home, master-less, exactly as the + * record and release paths already expect. + * + * Returns the retained slave fd for the caller to close, or -1. + */ +static int pty_keepalive_retire_stale_locked(int slot) +{ + int slave = pty_keepalive_table[slot].slave_host_fd; + pty_keepalive_table[slot].slave_host_fd = PTY_KEEPALIVE_FREE; + pty_keepalive_table[slot].stale_open_once = false; + return slave; +} + static uint32_t pty_extract_pts_num(const char *slave_path) { /* macOS canonical slave paths are /dev/ttysNNN with a decimal tail. Read @@ -1756,6 +1967,7 @@ static int pty_keepalive_register_locked(int master_host_fd, uint32_t linux_pts_num, const char *slave_path, bool stale_open_once, + bool fresh_segment, uint32_t *existing_pts_num) { int empty_slot = -1; @@ -1806,6 +2018,15 @@ static int pty_keepalive_register_locked(int master_host_fd, if (slot < 0) return PTY_REG_FULL; } + /* Reusing a stale-path slot inherits its mapping; hand it back before the + * fields below are overwritten, or the reference and any slaves it still + * counted would be stranded in the segment. + */ + pty_shared_detach(pty_keepalive_table[slot].shared, + pty_keepalive_table[slot].slave_path, + pty_keepalive_table[slot].guest_slave_count); + pty_keepalive_table[slot].shared = NULL; + pty_keepalive_table[slot].master_host_fd = master_host_fd; pty_keepalive_table[slot].guest_slave_count = 0; pty_keepalive_table[slot].guest_slave_seen = false; @@ -1820,6 +2041,16 @@ static int pty_keepalive_register_locked(int master_host_fd, PTY_SLAVE_PATH_MAX); else pty_keepalive_table[slot].slave_path[0] = '\0'; + + /* Only a pty the host just handed us gets a new segment. Every other + * registration -- a dup of a live master, an SCM_RIGHTS adopt, a + * fork-restore -- is one more reference to a pty that other processes may + * already be accounting for, and must join their segment. Discarding it + * would split the aliases onto separate counters, so slaves opened through + * one would be invisible to the other and the hangup would be lost. + */ + pty_keepalive_table[slot].shared = + pty_shared_attach(pty_keepalive_table[slot].slave_path, fresh_segment); return PTY_REG_INSERTED; } @@ -1834,12 +2065,13 @@ static int pty_keepalive_register(int master_host_fd, int slave_host_fd, uint32_t linux_pts_num, const char *slave_path, - bool stale_open_once) + bool stale_open_once, + bool fresh_segment) { pty_keepalive_lock_acquire(); - int rc = pty_keepalive_register_locked(master_host_fd, slave_host_fd, - linux_pts_num, slave_path, - stale_open_once, NULL); + int rc = pty_keepalive_register_locked( + master_host_fd, slave_host_fd, linux_pts_num, slave_path, + stale_open_once, fresh_segment, NULL); pthread_mutex_unlock(&pty_keepalive_lock); if (rc == PTY_REG_FULL) { errno = ENOSPC; @@ -1957,8 +2189,12 @@ uint32_t proc_pty_master_adopt(int guest_fd) goto out; } uint32_t existing_pts = UINT32_MAX; - int rc = pty_keepalive_register_locked(canonical_host_fd, slave, pts_num, - slave_path, false, &existing_pts); + /* Adopting a master elfuse did not open: the pty already exists and other + * processes may hold its segment, so join rather than replace. + */ + int rc = + pty_keepalive_register_locked(canonical_host_fd, slave, pts_num, + slave_path, false, false, &existing_pts); pthread_mutex_unlock(&fd_lock); pthread_mutex_unlock(&pty_keepalive_lock); if (rc == PTY_REG_FULL) { @@ -2066,19 +2302,93 @@ static void pty_guest_slave_release_locked(int slave_host_fd) uint32_t pts_num = pty_guest_slave_table[i].linux_pts_num; pty_guest_slave_table[i].slave_host_fd = PTY_KEEPALIVE_FREE; for (int k = 0; k < PTY_KEEPALIVE_MAX; k++) { - if (pty_keepalive_table[k].master_host_fd == PTY_KEEPALIVE_FREE) + /* A slot whose master already closed still owns this slave's + * accounting: the guest can drop the master and keep the slave as + * its stdio, and that slave has to be able to give its count back. + * Matching on the retained pts number covers both states; a fully + * cleared slot has neither a path nor a mapping and cannot match. + */ + if (pty_keepalive_table[k].slave_path[0] == '\0') continue; if (pty_keepalive_table[k].linux_pts_num != pts_num) continue; - if (pty_keepalive_table[k].guest_slave_count > 0) + if (pty_keepalive_table[k].guest_slave_count > 0) { pty_keepalive_table[k].guest_slave_count--; + pty_diag( + "pty: -slave pts=%u hostfd=%d local=%d shared=%d", pts_num, + slave_host_fd, pty_keepalive_table[k].guest_slave_count, + pty_keepalive_table[k].shared + ? atomic_load( + &pty_keepalive_table[k].shared->slave_count) - + 1 + : -1); + if (pty_keepalive_table[k].shared) + atomic_fetch_sub( + &pty_keepalive_table[k].shared->slave_count, 1); + } break; } break; } } -void proc_pty_note_guest_slave(int slave_host_fd, uint32_t linux_pts_num) +/* Put a slave fd on this process's books and credit its master. Caller holds + * the lock. + * + * bump_shared is false only for a slave inherited through fork: the parent + * already added it to the shared count on the child's behalf (see + * proc_pty_fork_parent_note_inherited), so counting it again here would double + * it. The local count still rises either way, since it is this process's + * contribution and what its closes and its detach subtract. + */ +static void pty_guest_slave_record_locked(int slave_host_fd, + uint32_t linux_pts_num, + bool bump_shared) +{ + for (int i = 0; i < PTY_GUEST_SLAVE_MAX; i++) { + if (pty_guest_slave_table[i].slave_host_fd != PTY_KEEPALIVE_FREE) + continue; + pty_guest_slave_table[i].slave_host_fd = slave_host_fd; + pty_guest_slave_table[i].linux_pts_num = linux_pts_num; + for (int k = 0; k < PTY_KEEPALIVE_MAX; k++) { + /* Match the rule the release path uses: a slot whose master has + * already closed still owns this pty's accounting. A fork-restored + * child routinely drops its copy of the master and only then opens + * /dev/pts/N, and requiring a live master here left that slave + * credited to nobody -- so the parent, still holding the master, + * never learned the shell had one. A fully cleared slot keeps + * neither a path nor a mapping and cannot match. + */ + if (pty_keepalive_table[k].slave_path[0] == '\0') + continue; + if (pty_keepalive_table[k].linux_pts_num != linux_pts_num) + continue; + pty_keepalive_table[k].guest_slave_count++; + pty_keepalive_table[k].guest_slave_seen = true; + pty_diag( + "pty: +slave pts=%u hostfd=%d bump_shared=%d local=%d " + "shared=%d", + linux_pts_num, slave_host_fd, (int) bump_shared, + pty_keepalive_table[k].guest_slave_count, + pty_keepalive_table[k].shared + ? atomic_load(&pty_keepalive_table[k].shared->slave_count) + + (bump_shared ? 1 : 0) + : -1); + if (pty_keepalive_table[k].shared) { + if (bump_shared) + atomic_fetch_add( + &pty_keepalive_table[k].shared->slave_count, 1); + atomic_store(&pty_keepalive_table[k].shared->seen, 1); + } + break; + } + break; + } +} + +static void pty_note_guest_slave(int slave_host_fd, + uint32_t linux_pts_num, + bool bump_shared) { if (slave_host_fd < 0) return; @@ -2091,25 +2401,184 @@ void proc_pty_note_guest_slave(int slave_host_fd, uint32_t linux_pts_num) * slot on reuse keeps that from inflating an unrelated pty's count. */ pty_guest_slave_release_locked(slave_host_fd); + pty_guest_slave_record_locked(slave_host_fd, linux_pts_num, bump_shared); + pthread_mutex_unlock(&pty_keepalive_lock); +} + +void proc_pty_note_guest_slave(int slave_host_fd, uint32_t linux_pts_num) +{ + pty_note_guest_slave(slave_host_fd, linux_pts_num, true); +} + +void proc_pty_fork_parent_note_inherited(void) +{ + /* fork duplicates every slave fd the guest holds, so the child's copies are + * live the instant fork returns. Count them here, in the parent, while the + * guest is still inside clone: leaving it to the child's own init loses the + * race against a parent that closes its copy immediately, which is exactly + * what openpty(3)-style terminal startup does. The pty would look hung up + * in that window and the terminal would see its shell die at startup. + */ + pty_keepalive_lock_acquire(); + /* The sentinel init is what makes an unused slot readable as free. Without + * it a table still in its BSS-zero state reads as PTY_GUEST_SLAVE_MAX + * occupied slots holding host fd 0, and every one of them would be counted + * as an inherited slave -- which is what a parent that never opened a slave + * itself does on its very first fork. + */ + pty_guest_slave_table_init_once(); for (int i = 0; i < PTY_GUEST_SLAVE_MAX; i++) { - if (pty_guest_slave_table[i].slave_host_fd != PTY_KEEPALIVE_FREE) + if (pty_guest_slave_table[i].slave_host_fd == PTY_KEEPALIVE_FREE) continue; - pty_guest_slave_table[i].slave_host_fd = slave_host_fd; - pty_guest_slave_table[i].linux_pts_num = linux_pts_num; + uint32_t pts_num = pty_guest_slave_table[i].linux_pts_num; for (int k = 0; k < PTY_KEEPALIVE_MAX; k++) { if (pty_keepalive_table[k].master_host_fd == PTY_KEEPALIVE_FREE) continue; - if (pty_keepalive_table[k].linux_pts_num != linux_pts_num) + if (pty_keepalive_table[k].linux_pts_num != pts_num) continue; - pty_keepalive_table[k].guest_slave_count++; - pty_keepalive_table[k].guest_slave_seen = true; + if (pty_keepalive_table[k].shared) + atomic_fetch_add(&pty_keepalive_table[k].shared->slave_count, + 1); break; } - break; } pthread_mutex_unlock(&pty_keepalive_lock); } +void proc_pty_dup_guest_slave_locked(int src_slave_host_fd, + int dst_slave_host_fd) +{ + if (src_slave_host_fd < 0 || dst_slave_host_fd < 0) + return; + pty_guest_slave_table_init_once(); + + uint32_t pts_num = UINT32_MAX; + for (int i = 0; i < PTY_GUEST_SLAVE_MAX; i++) { + if (pty_guest_slave_table[i].slave_host_fd == src_slave_host_fd) { + pts_num = pty_guest_slave_table[i].linux_pts_num; + break; + } + } + if (pts_num == UINT32_MAX) + return; /* not a tracked slave; nothing to mirror */ + + /* The dup is a live reference to the same slave, so it has to be counted + * like the open that produced the source. Only open() used to register, so + * a terminal that dup2()s its slave onto stdin/stdout/stderr and closes the + * original left the count at zero with three references still open -- the + * master then reported a hangup with the shell still running. + */ + pty_guest_slave_release_locked(dst_slave_host_fd); + pty_guest_slave_record_locked(dst_slave_host_fd, pts_num, true); +} + +void proc_pty_release_process_slaves(void) +{ + /* Hand back every slave this process still holds, at process teardown. + * + * Per-fd cleanup cannot be relied on for this: a shell exiting normally + * never closes its stdio, the kernel does, so the slaves backing fds 0/1/2 + * leave no close hook behind. Without this the shared count keeps a + * departed shell's slaves forever and the master never reports the hangup + * its terminal is waiting on -- the "window stays open after exit" case. + * + * A process killed outright still cannot run this, and leaks its + * contribution. That is bounded: the host pty is only recycled once every + * fd on it is gone, and the next master to claim that path starts a fresh + * segment (see pty_shared_attach), so the stale count is discarded rather + * than inherited. + */ + pty_keepalive_lock_acquire(); + for (int i = 0; i < PTY_KEEPALIVE_MAX; i++) { + if (!pty_keepalive_table[i].shared) + continue; + pty_shared_detach(pty_keepalive_table[i].shared, + pty_keepalive_table[i].slave_path, + pty_keepalive_table[i].guest_slave_count); + pty_keepalive_table[i].shared = NULL; + pty_keepalive_table[i].guest_slave_count = 0; + } + pthread_mutex_unlock(&pty_keepalive_lock); +} + +void proc_pty_adopt_inherited_slaves(void) +{ + /* A guest fork hands the child every slave fd the parent had open, but the + * table that maps a host fd back to its pty is per-process and does not + * travel, so those inherited slaves were counted by nobody. The parent then + * closes its own copy -- exactly what openpty(3)-style startup does -- the + * count falls to zero while the child's shell still holds a live slave, and + * the master reports a hangup the instant the terminal window appears. + * + * Recover the mapping from the host instead of shipping more state: a pty + * slave is a char device whose rdev matches the slave path recorded in the + * keepalive entry, which the child has just restored. Runs in the forked + * child's single-threaded init, after the fd table and the keepalives. + */ + struct { + uint32_t pts_num; + dev_t rdev; + int skip_slave_fd; + int skip_master_fd; + } ptys[PTY_KEEPALIVE_MAX]; + int npty = 0; + + pty_keepalive_lock_acquire(); + for (int i = 0; i < PTY_KEEPALIVE_MAX && npty < PTY_KEEPALIVE_MAX; i++) { + if (pty_keepalive_table[i].master_host_fd == PTY_KEEPALIVE_FREE) + continue; + if (pty_keepalive_table[i].slave_path[0] == '\0') + continue; + struct stat st; + if (stat(pty_keepalive_table[i].slave_path, &st) != 0 || + !S_ISCHR(st.st_mode)) + continue; + ptys[npty].pts_num = pty_keepalive_table[i].linux_pts_num; + ptys[npty].rdev = st.st_rdev; + ptys[npty].skip_slave_fd = pty_keepalive_table[i].slave_host_fd; + ptys[npty].skip_master_fd = pty_keepalive_table[i].master_host_fd; + npty++; + } + pthread_mutex_unlock(&pty_keepalive_lock); + if (npty == 0) + return; + + /* Snapshot the host fds before matching: proc_pty_note_guest_slave takes + * pty_keepalive_lock, which sorts before fd_lock, so neither lock can be + * held while calling it. + */ + int host_fds[FD_TABLE_SIZE]; + int nfd = 0; + pthread_mutex_lock(&fd_lock); + for (int gfd = 0; gfd < FD_TABLE_SIZE; gfd++) { + if (fd_table[gfd].type == FD_CLOSED || fd_table[gfd].host_fd < 0) + continue; + host_fds[nfd++] = fd_table[gfd].host_fd; + } + pthread_mutex_unlock(&fd_lock); + + for (int i = 0; i < nfd; i++) { + struct stat st; + if (fstat(host_fds[i], &st) != 0 || !S_ISCHR(st.st_mode)) + continue; + for (int p = 0; p < npty; p++) { + if (st.st_rdev != ptys[p].rdev) + continue; + /* elfuse's own keepalive slave is not a guest slave, and the + * master never matches the slave's rdev but is cheap to exclude. + */ + if (host_fds[i] == ptys[p].skip_slave_fd || + host_fds[i] == ptys[p].skip_master_fd) + break; + /* Local books only: the parent already counted these copies into + * the shared total before fork returned. + */ + pty_note_guest_slave(host_fds[i], ptys[p].pts_num, false); + break; + } + } +} + void proc_pty_slave_fd_closed(int host_fd) { if (host_fd < 0) @@ -2120,7 +2589,39 @@ void proc_pty_slave_fd_closed(int host_fd) pthread_mutex_unlock(&pty_keepalive_lock); } -bool proc_pty_master_hung_up(int guest_fd) +/* Whether this slot's pty has no guest slave left. Reads the shared segment + * when one is mapped, so a slave held by another process in the fork family + * counts; falls back to the per-process view when it is not. Caller holds + * pty_keepalive_lock. + */ +static bool pty_slot_hung_up_locked(int slot) +{ + pty_shared_t *sh = pty_keepalive_table[slot].shared; + bool hung_up; + if (sh) + hung_up = + atomic_load(&sh->seen) != 0 && atomic_load(&sh->slave_count) <= 0; + else + hung_up = pty_keepalive_table[slot].guest_slave_seen && + pty_keepalive_table[slot].guest_slave_count == 0; + + /* Only on the way to reporting one: the negative answer is the steady + * state and every poll would log it. This subsystem spans processes, so + * without a record of which side saw what a wrong verdict is very hard to + * place after the fact. + */ + if (hung_up) + pty_diag("pty: HANGUP pts=%u seen=%d shared=%d local=%d/%d path=%s", + pty_keepalive_table[slot].linux_pts_num, + sh ? atomic_load(&sh->seen) : -1, + sh ? atomic_load(&sh->slave_count) : -1, + (int) pty_keepalive_table[slot].guest_slave_seen, + pty_keepalive_table[slot].guest_slave_count, + pty_keepalive_table[slot].slave_path); + return hung_up; +} + +bool proc_pty_master_hung_up(int guest_fd, uint64_t expect_generation) { /* Keyed on the guest fd rather than a host one: callers reach the master * through host_fd_ref, which hands out a dup, and the keepalive table is @@ -2129,6 +2630,12 @@ bool proc_pty_master_hung_up(int guest_fd) fd_entry_t snap; if (!fd_snapshot(guest_fd, &snap)) return false; + /* The caller resolved this guest fd earlier; re-resolving it here reopens + * the close-and-reuse window. Reject a slot that has been recycled since, + * so the hangup is never charged to an unrelated file. + */ + if (snap.generation != expect_generation) + return false; int master_host_fd = snap.host_fd; if (master_host_fd < 0) return false; @@ -2137,8 +2644,7 @@ bool proc_pty_master_hung_up(int guest_fd) for (int i = 0; i < PTY_KEEPALIVE_MAX; i++) { if (pty_keepalive_table[i].master_host_fd != master_host_fd) continue; - hung_up = pty_keepalive_table[i].guest_slave_seen && - pty_keepalive_table[i].guest_slave_count == 0; + hung_up = pty_slot_hung_up_locked(i); break; } pthread_mutex_unlock(&pty_keepalive_lock); @@ -2190,7 +2696,7 @@ static int pty_open_slave(uint32_t linux_pts_num, int linux_flags) */ size_t len = strlen(pty_keepalive_table[stale_hit].slave_path); if (len >= sizeof(host_path)) { - int retained_slave = pty_keepalive_clear_slot_locked(stale_hit); + int retained_slave = pty_keepalive_retire_stale_locked(stale_hit); pthread_mutex_unlock(&pty_keepalive_lock); if (retained_slave >= 0) close(retained_slave); @@ -2208,7 +2714,7 @@ static int pty_open_slave(uint32_t linux_pts_num, int linux_flags) if (strncmp(pty_keepalive_table[i].slave_path, host_path, PTY_SLAVE_PATH_MAX) != 0) continue; - int retained_slave = pty_keepalive_clear_slot_locked(i); + int retained_slave = pty_keepalive_retire_stale_locked(i); if (retained_slave >= 0 && nretained < PTY_KEEPALIVE_MAX) retained_slaves[nretained++] = retained_slave; } @@ -2317,9 +2823,9 @@ void proc_pty_dup_keepalive_locked(int src_master_host_fd, close(dst_slave); return; } - int rc = - pty_keepalive_register_locked(dst_master_host_fd, dst_slave, - src_pts_num, src_slave_path, false, NULL); + int rc = pty_keepalive_register_locked(dst_master_host_fd, dst_slave, + src_pts_num, src_slave_path, false, + /*fresh_segment=*/false, NULL); if (rc != PTY_REG_INSERTED) { /* Table full or duplicate entry for dst_master_host_fd; drop the * redundant slave. Duplicate is unexpected: dst is a freshly-duped host @@ -2347,9 +2853,14 @@ void proc_pty_close_keepalive(int master_host_fd) * /dev/pts/N open after close(master). pty_open_slave consumes and * closes it on the first translated open attempt. */ + /* Only the master goes away here. Any slave fd this process still + * holds stays open and keeps counting: closing the master does not + * close the slaves, and a terminal's child routinely drops its copy + * of the master while holding the slave as its stdio. Retiring the + * count here would report a hangup with the shell still running. + * The slaves decrement themselves as they close. + */ pty_keepalive_table[slot].master_host_fd = PTY_KEEPALIVE_FREE; - pty_keepalive_table[slot].guest_slave_count = 0; - pty_keepalive_table[slot].guest_slave_seen = false; } else { slave = pty_keepalive_clear_slot_locked(slot); } @@ -2388,11 +2899,12 @@ static int pty_keepalive_register_recycled(int master_host_fd, int slave_host_fd, uint32_t linux_pts_num, const char *slave_path, - bool stale_open_once) + bool stale_open_once, + bool fresh_segment) { proc_pty_expire_stale_by_path(slave_path); return pty_keepalive_register(master_host_fd, slave_host_fd, linux_pts_num, - slave_path, stale_open_once); + slave_path, stale_open_once, fresh_segment); } int proc_pty_snapshot_keepalive(proc_pty_ipc_entry_t *out_entries, @@ -2452,7 +2964,8 @@ void proc_pty_restore_keepalive(int master_host_fd, */ errno = 0; if (pty_keepalive_register_recycled(master_host_fd, slave_host_fd, - linux_pts_num, slave_path, true) < 0 || + linux_pts_num, slave_path, true, + /*fresh_segment=*/false) < 0 || errno == EEXIST) goto drop; return; @@ -2508,8 +3021,12 @@ static int pty_open_master(int linux_flags) return -1; } errno = 0; + /* The host just allocated this pty, so nothing live can be using a segment + * under its name; any leftover is state a process died holding. + */ if (pty_keepalive_register_recycled(master, slave, linux_pts_num, - slave_path, false) < 0) { + slave_path, false, + /*fresh_segment=*/true) < 0) { close(slave); close(master); errno = EMFILE; diff --git a/src/runtime/procemu.h b/src/runtime/procemu.h index 176cb61a..764b6d67 100644 --- a/src/runtime/procemu.h +++ b/src/runtime/procemu.h @@ -122,11 +122,49 @@ void proc_pty_note_guest_slave(int slave_host_fd, uint32_t linux_pts_num); */ void proc_pty_slave_fd_closed(int host_fd); +/* Count the pty slave fds a forked child inherited from its parent. + * + * The host-fd-to-pty mapping is per-process and does not cross the fork, so + * without this an inherited slave is counted by nobody: the parent closes its + * own copy, the count reaches zero while the child's shell still holds a live + * slave, and the master reports a hangup as soon as the terminal starts. Call + * from the fork child once its fd table and keepalives are both restored. + */ +void proc_pty_adopt_inherited_slaves(void); + +/* Return this process's outstanding pty slave count at teardown. + * + * A guest that exits normally never closes its stdio -- the kernel does -- so + * the slaves behind fds 0/1/2 never reach the per-fd close hook. Without this + * the shared count keeps a departed shell's slaves forever and the master never + * reports the hangup its terminal is waiting on. Call once from the process + * cleanup path; idempotent. + */ +void proc_pty_release_process_slaves(void); + +/* Count the slave fds a fork is about to duplicate into the child. + * + * Called in the parent while the guest is still inside clone, because the child + * cannot do it for itself in time: a parent that closes its own copy the moment + * fork returns -- openpty(3)-style terminal startup -- would otherwise drive + * the count to zero before the child's init runs, and the master would report a + * hangup with the shell alive. + */ +void proc_pty_fork_parent_note_inherited(void); + /* True when the guest has closed every slave fd it held for this master, the - * state Linux reports as a hangup: poll gives POLLHUP and read gives EIO. - * elfuse's own keepalive slave stays open, so the host cannot answer this. + * state Linux reports as a hangup: poll gives POLLHUP, epoll gives EPOLLHUP, + * and read gives EIO. elfuse's own keepalive slave stays open, so the host + * cannot answer this. + * + * expect_generation is the fd generation the caller pinned when it resolved + * guest_fd. The lookup re-resolves the guest fd, so without that witness a + * close-and-reuse in the window would report the hangup against whatever file + * now occupies the slot. Pass fd_current_generation(guest_fd) captured at + * resolve time; a mismatch (including the 0 returned for a closed slot) reports + * no hangup. */ -bool proc_pty_master_hung_up(int guest_fd); +bool proc_pty_master_hung_up(int guest_fd, uint64_t expect_generation); /* Caller-locked variant of pty keepalive duplication. Brackets the host * fd_snapshot_and_dup and the keepalive mirror under one pty_keepalive_lock @@ -139,6 +177,19 @@ void proc_pty_unlock_for_dup(void); void proc_pty_dup_keepalive_locked(int src_master_host_fd, int dst_master_host_fd); +/* Slave-side counterpart of proc_pty_dup_keepalive_locked: mirror a guest pty + * slave onto the host fd a dup just produced. + * + * open() and the TIOCGPTPEER ioctl are the two paths that register a slave; + * every further reference dup2 creates went uncounted. A terminal that dup2()s + * its slave onto stdin/stdout/stderr and closes the original then drove the + * count to zero with three live references left, and the master reported a + * hangup while the shell was still running. Caller must hold the dup bracket + * (proc_pty_lock_for_dup). + */ +void proc_pty_dup_guest_slave_locked(int src_slave_host_fd, + int dst_slave_host_fd); + /* Return the captured Linux pts number for a host master fd, or UINT32_MAX when * no keepalive is registered. Lets sys_ioctl TIOCGPTN report the value * /dev/pts/N opens / stats round-trip through, instead of independently parsing diff --git a/src/syscall/fdtable.c b/src/syscall/fdtable.c index 186c403e..e6d61663 100644 --- a/src/syscall/fdtable.c +++ b/src/syscall/fdtable.c @@ -319,7 +319,7 @@ int fd_alloc_dir_at(int fd, pthread_mutex_lock(&fd_lock); if (fd_table[fd].type != FD_CLOSED) { old = fd_table[fd]; - epoll_note_fd_closed(fd); + epoll_note_fd_closed(fd, old.ofd_id); } fd_init_entry(fd, type, host_fd, cleanup); fd_table[fd].dir = dir; @@ -438,7 +438,7 @@ int fd_alloc_at(int fd, * this fd number without routing through fd_mark_closed_unlocked, so * clear its epoll registrations here too (see epoll_note_fd_closed). */ - epoll_note_fd_closed(fd); + epoll_note_fd_closed(fd, old.ofd_id); } fd_init_entry(fd, type, host_fd, cleanup); if (out_gen) @@ -484,6 +484,8 @@ int fd_alloc_at_relaxed(int fd, */ void fd_mark_closed_unlocked(int fd) { + uint64_t closing_ofd_id = fd_table[fd].ofd_id; + /* Clear before publishing FD_CLOSED/free. The EL1 urandom read fast path * intentionally avoids fd_lock, so it must not observe a stale urandom bit * after this slot has become invalid or reusable. @@ -506,7 +508,7 @@ void fd_mark_closed_unlocked(int fd) * just-closed epoll fd skips itself; caller holds fd_lock (or is * single-threaded on the relaxed path). */ - epoll_note_fd_closed(fd); + epoll_note_fd_closed(fd, closing_ofd_id); } void fd_mark_closed(int fd) @@ -603,6 +605,14 @@ bool fd_snapshot(int guest_fd, fd_entry_t *out) return ok; } +uint64_t fd_current_generation(int guest_fd) +{ + fd_entry_t snap; + if (!fd_snapshot(guest_fd, &snap)) + return 0; + return snap.generation; +} + int fd_snapshot_and_dup(int guest_fd, fd_entry_t *out) { out->type = FD_CLOSED; diff --git a/src/syscall/fs.c b/src/syscall/fs.c index 18c36f0a..0a8c6254 100644 --- a/src/syscall/fs.c +++ b/src/syscall/fs.c @@ -965,6 +965,11 @@ static int duplicate_guest_fd(int src_fd, * race and leak the slave fd. No-op when the source has no keepalive. */ proc_pty_dup_keepalive_locked(src_snap.host_fd, new_host_fd); + /* Same reasoning for the slave side: the alias is a live reference to the + * pty and must be on the books before the guest fd is published, or the + * source's close will retire the only counted reference. + */ + proc_pty_dup_guest_slave_locked(src_snap.host_fd, new_host_fd); proc_pty_unlock_for_dup(); int new_type = (src_snap.type == FD_STDIO) ? FD_REGULAR : src_snap.type; @@ -980,9 +985,13 @@ static int duplicate_guest_fd(int src_fd, errno = EBADF; /* fd_cleanup_entry never ran on new_host_fd (no guest fd was - * registered), so the keepalive must be dropped explicitly here. + * registered), so both halves of the pty bookkeeping must be dropped + * explicitly here. Leaving the slave counted would keep a host fd that + * is about to be closed on the books, and the master would never see + * its last slave go. */ proc_pty_close_keepalive(new_host_fd); + proc_pty_slave_fd_closed(new_host_fd); close_keep_errno(new_host_fd); return -1; } diff --git a/src/syscall/internal.h b/src/syscall/internal.h index 55d4162a..9914a312 100644 --- a/src/syscall/internal.h +++ b/src/syscall/internal.h @@ -153,6 +153,16 @@ int fd_to_host(int guest_fd); */ bool fd_snapshot(int guest_fd, fd_entry_t *out); +/* Read the generation currently published for a guest fd. + * + * Returns 0 when the slot is closed or out of range. Generations start at 1 and + * only ever increase, so 0 never compares equal to a live one and a slot that + * changed can never compare equal to an earlier reading. Use this to re-check a + * generation pinned earlier, not to pin one against a host fd resolved in a + * separate window -- see host_fd_ref_open_io_gen() for that. + */ +uint64_t fd_current_generation(int guest_fd); + /* Snapshot an fd entry AND dup its host fd in a single fd_lock critical * section. Eliminates the TOCTOU window between reading the type/metadata and * duplicating the host fd in the dup(2) path. @@ -402,6 +412,56 @@ static inline int64_t host_fd_ref_open_io(guest_fd_t guest_fd, return 0; } +/* host_fd_ref_open_io() that also reports the fd generation the reference was + * resolved against. + * + * The generation is captured in the same fd_lock window as the host fd, so the + * two always describe one open file. Callers that later re-resolve the guest fd + * -- the pty hangup checks -- need exactly that pairing: a generation sampled + * in a separate window can already belong to a replacement file while the + * reference still points at the original, and the replacement's state would + * then be reported against the original. fd_entry_t.host_fd is only ever + * written together with a fresh generation, so the generation alone pins the + * identity of the file behind ref->fd. + * + * *out_gen is 0 on failure. Returns 0 on success, -LINUX_EBADF otherwise. + */ +static inline int64_t host_fd_ref_open_io_gen(guest_fd_t guest_fd, + host_fd_ref_t *ref, + uint64_t *out_gen) +{ + ref->fd = -1; + ref->owned = false; + *out_gen = 0; + + fd_entry_t snap; + if (thread_is_single_active()) { + /* No sibling can race the slot, so a plain snapshot is already + * consistent with the borrowed host fd. + */ + if (!fd_snapshot(guest_fd, &snap) || snap.type == FD_PATH || + snap.host_fd < 0) + return -LINUX_EBADF; + ref->fd = snap.host_fd; + *out_gen = snap.generation; + return 0; + } + + int host_fd = fd_snapshot_and_dup(guest_fd, &snap); + if (host_fd < 0) + return -LINUX_EBADF; + if (snap.type == FD_PATH) { + int saved_errno = errno; + close(host_fd); + errno = saved_errno; + return -LINUX_EBADF; + } + ref->fd = host_fd; + ref->owned = true; + *out_gen = snap.generation; + return 0; +} + /* iov limits shared between readv/writev/preadv/pwritev and sendmsg/recvmsg. * SYSCALL_IOV_MAX matches the Linux UIO_MAXIOV cap; SYSCALL_IOV_STACK_MAX keeps * the typical case on the call-site stack. diff --git a/src/syscall/io.c b/src/syscall/io.c index ec5a2724..87ede1b9 100644 --- a/src/syscall/io.c +++ b/src/syscall/io.c @@ -845,11 +845,41 @@ static int64_t host_fd_ref_open_checked(int guest_fd, return host_fd_ref_open_io(guest_fd, ref); } +/* True when a read on this pty master must fail with EIO. + * + * Linux fails every read variant once the master has hung up, not just + * read(2). Only after the queue drains: a shell that printed on its way out + * leaves that output behind, and Linux hands it over before reporting the + * hangup, so deciding on the hangup first would swallow it. + * + * Without this the host read simply blocks -- elfuse's keepalive slave keeps + * the pty alive from its point of view -- so a terminal that drains its master + * with readv() gets the POLLHUP, calls readv, and wedges there forever with its + * window still open. + */ +static bool pty_read_hangs_up(int guest_fd, uint64_t gen, int host_fd) +{ + if (!proc_pty_master_hung_up(guest_fd, gen)) + return false; + struct pollfd drain = {.fd = host_fd, .events = POLLIN}; + return poll(&drain, 1, 0) <= 0 || !(drain.revents & POLLIN); +} + static int64_t host_fd_ref_open_regular_io(int guest_fd, host_fd_ref_t *ref) { return host_fd_ref_open_io(guest_fd, ref); } +/* host_fd_ref_open_regular_io() that also pins the generation the reference was + * resolved against, in the same fd_lock window. See host_fd_ref_open_io_gen(). + */ +static int64_t host_fd_ref_open_regular_io_gen(int guest_fd, + host_fd_ref_t *ref, + uint64_t *out_gen) +{ + return host_fd_ref_open_io_gen(guest_fd, ref, out_gen); +} + static int64_t proc_try_read_intercept(int fd, int host_fd, void *buf, @@ -1051,8 +1081,13 @@ int64_t sys_read(guest_t *g, int fd, uint64_t buf_gva, uint64_t count) return urandom_read(g, fd, buf_gva, count); } + /* Pin the generation in the same fd_lock window as the host fd. The pty + * hangup check below re-resolves the guest fd, and this is the witness that + * the slot still holds the very file this read resolved. + */ host_fd_ref_t host_ref; - int64_t err = host_fd_ref_open_regular_io(fd, &host_ref); + uint64_t read_gen; + int64_t err = host_fd_ref_open_regular_io_gen(fd, &host_ref, &read_gen); if (err < 0) return err; @@ -1080,12 +1115,9 @@ int64_t sys_read(guest_t *g, int fd, uint64_t buf_gva, uint64_t count) * leaves that output queued, and Linux hands it over before reporting the * hangup. Deciding on the hangup first would swallow it. */ - if (proc_pty_master_hung_up(fd)) { - struct pollfd drain = {.fd = host_ref.fd, .events = POLLIN}; - if (poll(&drain, 1, 0) <= 0 || !(drain.revents & POLLIN)) { - host_fd_ref_close(&host_ref); - return -LINUX_EIO; - } + if (pty_read_hangs_up(fd, read_gen, host_ref.fd)) { + host_fd_ref_close(&host_ref); + return -LINUX_EIO; } off_t offset = lseek(host_ref.fd, 0, SEEK_CUR); @@ -1432,7 +1464,8 @@ int64_t sys_readv(guest_t *g, int fd, uint64_t iov_gva, int iovcnt) } host_fd_ref_t host_ref; - int64_t err = host_fd_ref_open_regular_io(fd, &host_ref); + uint64_t readv_gen; + int64_t err = host_fd_ref_open_regular_io_gen(fd, &host_ref, &readv_gen); if (err < 0) return err; @@ -1449,6 +1482,19 @@ int64_t sys_readv(guest_t *g, int fd, uint64_t iov_gva, int iovcnt) return err < 0 ? err : 0; } + /* Same hangup rule as sys_read: a terminal draining its master with readv + * must be told the shell is gone, or it blocks here forever. + * + * After the zero-payload branch on purpose: a vector that can consume + * nothing reads zero bytes on Linux whatever the pty's state, so deciding + * the hangup first would turn that into a spurious EIO. + */ + if (pty_read_hangs_up(fd, readv_gen, host_ref.fd)) { + host_iov_free(&host_iov); + host_fd_ref_close(&host_ref); + return -LINUX_EIO; + } + off_t offset = lseek(host_ref.fd, 0, SEEK_CUR); if (offset >= 0) { int64_t intercepted = proc_try_readv_intercept( @@ -1960,7 +2006,8 @@ int64_t sys_ioctl(guest_t *g, int fd, uint64_t request, uint64_t arg) } host_fd_ref_t host_ref; - int64_t err = host_fd_ref_open_regular_io(fd, &host_ref); + uint64_t ioctl_gen; + int64_t err = host_fd_ref_open_regular_io_gen(fd, &host_ref, &ioctl_gen); if (err < 0) return err; int host_fd = host_ref.fd; @@ -2316,6 +2363,13 @@ int64_t sys_ioctl(guest_t *g, int fd, uint64_t request, uint64_t arg) host_fd_ref_close(&host_ref); return -LINUX_EINVAL; } + /* Resolve the Linux pts number before opening, the same way TIOCGPTN + * does: the slave handed out here counts toward the master's hangup + * accounting, and that table is keyed by pts number. Pass the guest fd + * so the adopt validates against the canonical (host_fd, generation) + * rather than this call's host_fd_ref dup. + */ + uint32_t pts_num = proc_pty_master_adopt(fd); char slave[64]; if (ptsname_r(host_fd, slave, sizeof(slave)) != 0) { host_fd_ref_close(&host_ref); @@ -2336,6 +2390,25 @@ int64_t sys_ioctl(guest_t *g, int fd, uint64_t request, uint64_t arg) return -LINUX_EMFILE; } + /* Record the slave against its master. Without this the master's + * guest_slave_seen stays false and a master whose only slave arrived + * this way never reports a hangup -- and TIOCGPTPEER has been the + * recommended way to reach the peer since Linux 4.13. Registered after + * fd_alloc so a failed alloc leaves nothing behind to retire. + * + * Only when the slot still holds the generation this ioctl resolved. + * proc_pty_master_adopt re-resolves the guest fd, so its pts_num + * describes the master the slot held at that moment, while ptsname_r + * and the open above used the host fd pinned on entry. Generations only + * ever increase, so an unchanged one here proves the slot never moved + * across either window and the two agree on one master. If it did move, + * pts_num belongs to a different pty and charging the slave to it would + * corrupt that master's accounting; dropping the record instead only + * forgoes a hangup report on a master the guest concurrently closed. + */ + if (pts_num != UINT32_MAX && fd_current_generation(fd) == ioctl_gen) + proc_pty_note_guest_slave(host_slave_fd, pts_num); + /* Track CLOEXEC + accmode in the guest table so exec honors them; the * host fd's own FD_CLOEXEC is per-descriptor and would be lost on the * dup that host_fd_ref hands multi-threaded callers. diff --git a/src/syscall/poll.c b/src/syscall/poll.c index 3823e6cd..c4cfd590 100644 --- a/src/syscall/poll.c +++ b/src/syscall/poll.c @@ -119,13 +119,20 @@ int64_t sys_ppoll(guest_t *g, struct pollfd host_fds[256]; host_fd_ref_t host_refs[256]; bool need_pollnval[256] = {false}; + /* Generation pinned per entry in the same fd_lock window as its host fd. + * The pty hangup checks below re-resolve the guest fd, so each needs a + * witness that the slot still holds the very file this poll resolved; 0 + * marks entries with nothing pinned and never matches a live generation. + */ + uint64_t guest_gen[256] = {0}; uint32_t invalid_count = 0; for (uint32_t i = 0; i < nfds; i++) { host_refs[i] = (host_fd_ref_t) {.fd = -1, .owned = false}; int guest_fd = guest_fds[i].fd; int host_fd = -1; if (guest_fd >= 0) { - if (host_fd_ref_open_io(guest_fd, &host_refs[i]) < 0) { + if (host_fd_ref_open_io_gen(guest_fd, &host_refs[i], + &guest_gen[i]) < 0) { need_pollnval[i] = true; invalid_count++; } else { @@ -252,8 +259,9 @@ int64_t sys_ppoll(guest_t *g, if (ret == 0) { bool hup_pending = false; for (uint32_t i = 0; i < nfds && !hup_pending; i++) - hup_pending = !need_pollnval[i] && guest_fds[i].fd >= 0 && - proc_pty_master_hung_up(guest_fds[i].fd); + hup_pending = + !need_pollnval[i] && guest_fds[i].fd >= 0 && + proc_pty_master_hung_up(guest_fds[i].fd, guest_gen[i]); if (hup_pending) break; } @@ -279,7 +287,7 @@ int64_t sys_ppoll(guest_t *g, for (uint32_t i = 0; i < nfds; i++) { if (need_pollnval[i] || guest_fds[i].fd < 0) continue; - if (!proc_pty_master_hung_up(guest_fds[i].fd)) + if (!proc_pty_master_hung_up(guest_fds[i].fd, guest_gen[i])) continue; if (host_fds[i].revents == 0) ret++; @@ -758,12 +766,14 @@ typedef struct { * file is gone and this stale entry must not drive * kevent against the reused host fd. */ - bool active; /* Registered in this instance */ - bool oneshot_armed; /* EPOLLONESHOT and event already fired, - * waiting for EPOLL_CTL_MOD re-arm. - * kqueue removed the event, so poll emulation prevents - * reporting but allow MOD. - */ + uint64_t ofd_id; /* Open file description identity captured at ADD/MOD. */ + bool active; /* Registered in this instance */ + bool oneshot_armed; /* EPOLLONESHOT and event already fired, + * waiting for EPOLL_CTL_MOD re-arm. + * kqueue removed the event, so poll emulation prevents + * reporting but allow MOD. + */ + bool pty_master; /* Registration is for a tracked pty master. */ } epoll_reg_t; /* Per-epoll-instance data, stored in fd_table[epfd].dir. Each instance has its @@ -788,9 +798,25 @@ typedef struct { * ready-list scans. Lock order: fd_lock -> lock (see internal.h). */ pthread_mutex_t lock; + int active_count; + int pty_master_count; epoll_reg_t regs[FD_TABLE_SIZE]; } epoll_instance_t; +static void epoll_reg_deactivate_locked(epoll_instance_t *inst, + epoll_reg_t *reg) +{ + if (reg->active && inst->active_count > 0) + inst->active_count--; + if (reg->pty_master && inst->pty_master_count > 0) + inst->pty_master_count--; + reg->active = false; + reg->oneshot_armed = false; + reg->pty_master = false; + reg->generation = 0; + reg->ofd_id = 0; +} + /* Count of live epoll instances. When zero, epoll_note_fd_closed() skips its * scan entirely -- the overwhelmingly common case (a process with no epoll fd * pays nothing on every close). Guarded by fd_lock, matching the fd_table scan. @@ -928,10 +954,19 @@ int epoll_dup_fd(int src_fd, * drops it automatically, and clearing the software state is what makes the * pwait active-check honest. */ -void epoll_note_fd_closed(int closed_fd) +void epoll_note_fd_closed(int closed_fd, uint64_t closed_ofd_id) { if (epoll_live_count == 0) return; + if (closed_ofd_id != 0) { + for (int fd = 0; fd < FD_TABLE_SIZE; fd++) { + if (fd == closed_fd) + continue; + if (fd_table[fd].type != FD_CLOSED && + fd_table[fd].ofd_id == closed_ofd_id) + return; + } + } for (int epfd = 0; epfd < FD_TABLE_SIZE; epfd++) { if (fd_table[epfd].type != FD_EPOLL) continue; @@ -943,10 +978,7 @@ void epoll_note_fd_closed(int closed_fd) * refcount is at least the table's reference and cannot be freed here. */ pthread_mutex_lock(&inst->lock); - epoll_reg_t *reg = &inst->regs[closed_fd]; - reg->active = false; - reg->oneshot_armed = false; - reg->generation = 0; + epoll_reg_deactivate_locked(inst, &inst->regs[closed_fd]); pthread_mutex_unlock(&inst->lock); } } @@ -1079,6 +1111,8 @@ int64_t sys_epoll_ctl(guest_t *g, int epfd, int op, int fd, uint64_t event_gva) goto out; } int target_host_fd = target_snap.host_fd; + bool target_pty_master = + proc_pty_master_pts_num(target_host_fd) != UINT32_MAX; /* Serialize all regs[] access and the paired kqueue mutation against a * concurrent close hook or a sibling epoll_ctl on the same instance. The @@ -1098,8 +1132,7 @@ int64_t sys_epoll_ctl(guest_t *g, int epfd, int op, int fd, uint64_t event_gva) */ if ((reg->active || reg->oneshot_armed) && reg->generation != target_snap.generation) { - reg->active = false; - reg->oneshot_armed = false; + epoll_reg_deactivate_locked(inst, reg); } if (op == LINUX_EPOLL_CTL_DEL) { @@ -1127,9 +1160,7 @@ int64_t sys_epoll_ctl(guest_t *g, int epfd, int op, int fd, uint64_t event_gva) } /* Ignore errors from EV_DELETE (fd might already be closed) */ kevent(epoll_ref.fd, changes, nchanges, NULL, 0, NULL); - reg->active = false; - /* Clear stale state for potential re-add */ - reg->oneshot_armed = false; + epoll_reg_deactivate_locked(inst, reg); } ret = 0; goto out_locked; @@ -1227,8 +1258,17 @@ int64_t sys_epoll_ctl(guest_t *g, int epfd, int op, int fd, uint64_t event_gva) reg->events = ev.events; reg->data = ev.data; reg->generation = target_snap.generation; + reg->ofd_id = target_snap.ofd_id; + if (!reg->active) + inst->active_count++; + if (target_pty_master && !reg->pty_master) + inst->pty_master_count++; + else if (!target_pty_master && reg->pty_master && + inst->pty_master_count > 0) + inst->pty_master_count--; reg->active = true; reg->oneshot_armed = false; + reg->pty_master = target_pty_master; ret = 0; @@ -1240,6 +1280,79 @@ int64_t sys_epoll_ctl(guest_t *g, int epfd, int op, int fd, uint64_t event_gva) return ret; } +/* Collect guest fds registered in this instance whose pty master has hung up. + * + * kqueue never reports this: elfuse holds a keepalive slave open for the + * master's whole life, so macOS still considers the pty live and stays silent. + * Without this an epoll-based terminal waits forever for a hangup that cannot + * arrive -- foot leaves its window open after the shell exits. + * + * Two-phase on purpose. regs[] needs inst->lock, but proc_pty_master_hung_up + * takes fd_lock, which sorts *before* inst->lock (see internal.h). Testing + * under the reg lock would invert that order against epoll_note_fd_closed, + * which holds fd_lock and then takes inst->lock. So candidates are snapshotted + * under the reg lock in bounded batches and tested once it is dropped. + * + * Returns the number of guest fds written to out_gfds, capped at max. + * out_gens receives the registration generation each hit was tested against, so + * the caller can re-verify it under inst->lock before acting: a sibling can + * EPOLL_CTL_DEL and re-ADD the same fd number while the lock is dropped, and + * stamping the hangup then would attach it to the new registration's data. + */ +static int epoll_collect_hung_up(epoll_instance_t *inst, + int *out_gfds, + uint64_t *out_gens, + int max) +{ + if (max <= 0) + return 0; + + enum { HUP_SCAN_BATCH = 64 }; + int cand_gfds[HUP_SCAN_BATCH]; + uint64_t cand_gens[HUP_SCAN_BATCH]; + int n = 0; + int seen = 0; + + for (int gfd = 0; gfd < FD_TABLE_SIZE && n < max;) { + int ncand = 0; + int active_count; + pthread_mutex_lock(&inst->lock); + if (inst->pty_master_count <= 0) { + pthread_mutex_unlock(&inst->lock); + break; + } + active_count = inst->active_count; + for (; gfd < FD_TABLE_SIZE && ncand < HUP_SCAN_BATCH && + seen < active_count; + gfd++) { + if (!inst->regs[gfd].active) + continue; + seen++; + if (inst->regs[gfd].oneshot_armed || !inst->regs[gfd].pty_master) + continue; + cand_gfds[ncand] = gfd; + /* Carry the generation the registration pinned at ADD/MOD, so a + * close+reopen into the same fd number cannot be mistaken for the + * registered master. + */ + cand_gens[ncand] = inst->regs[gfd].generation; + ncand++; + } + pthread_mutex_unlock(&inst->lock); + if (ncand == 0 && seen >= active_count) + break; + + for (int i = 0; i < ncand && n < max; i++) { + if (!proc_pty_master_hung_up(cand_gfds[i], cand_gens[i])) + continue; + out_gfds[n] = cand_gfds[i]; + out_gens[n] = cand_gens[i]; + n++; + } + } + return n; +} + int64_t sys_epoll_pwait(guest_t *g, int epfd, uint64_t events_gva, @@ -1288,20 +1401,17 @@ int64_t sys_epoll_pwait(guest_t *g, ts.tv_nsec = (timeout_ms % 1000) * 1000000L; } -epoll_retry:; - - /* For indefinite waits, register the wakeup pipe with the kqueue so - * exit_group/futex/signal requests can interrupt threads blocked in - * kevent(). + /* A hangup that is already pending must not wait out the caller's timeout. + * kqueue will never report it, so a finite epoll_wait would otherwise block + * to its deadline before the stamping below runs, and an indefinite one + * would burn a needless 200ms slice. Collect whatever else is ready without + * blocking and fall straight through to the stamp. */ - bool added_wakeup = false; - if (!has_timeout && wakeup_pipe_rd >= 0) { - struct kevent wake_ev; - EV_SET(&wake_ev, wakeup_pipe_rd, EVFILT_READ, EV_ADD | EV_ONESHOT, 0, 0, - (void *) (uintptr_t) -1); - kevent(epoll_ref.fd, &wake_ev, 1, NULL, 0, NULL); - added_wakeup = true; - } + int hup_probe; + uint64_t hup_probe_gen; + bool hup_ready = + epoll_collect_hung_up(inst, &hup_probe, &hup_probe_gen, 1) > 0; + struct timespec zero_ts = {.tv_sec = 0, .tv_nsec = 0}; /* Collect kqueue events. For indefinite waits, use a short timeout and loop * so exit_group can interrupt. Cap maxevents before multiply to avoid @@ -1312,45 +1422,58 @@ epoll_retry:; int cap = maxevents * 2; /* Each epoll fd can produce 2 kevents */ if (cap > 256) cap = 256; - /* Reserve one slot for the wakeup pipe event */ - if (added_wakeup && cap < 256) - cap++; struct kevent kevents[256]; struct timespec poll_ts = {.tv_sec = 0, .tv_nsec = 200000000L}; /* 200ms */ int nready; do { nready = kevent(epoll_ref.fd, NULL, 0, kevents, cap, - has_timeout ? &ts : &poll_ts); + hup_ready ? &zero_ts : (has_timeout ? &ts : &poll_ts)); + if (nready > 0) { + } - if (proc_exit_group_requested() || futex_interrupt_consume() || - signal_pending_interruption(NULL)) { + /* Evaluated stepwise only to name the one that fired; the guards + * preserve the short-circuit order, so futex_interrupt_consume() still + * runs exactly when it did as a single ||-chain. + */ + /* Ready events outrank an interruption. Linux ep_poll() tests + * ep_events_available() and jumps to send_events before it ever looks + * at signal_pending(), so EINTR is the answer only for a wait that + * produced nothing. + * + * Returning EINTR while holding a ready fd loses it for good in + * practice: kqueue re-reports it on the next call, but the same pending + * signal is still there, so the guest is handed EINTR forever and never + * drains the fd. foot hit exactly that -- a SIGCHLD it had a handler + * for but had not yet run left its Wayland socket readable and + * undelivered, and it spun at 100% CPU without ever drawing a window. + * + * exit_group still wins outright: the process is going away and there + * is nothing to deliver events to. + */ + bool interrupted = proc_exit_group_requested(); + if (!interrupted && nready <= 0) + interrupted = + futex_interrupt_consume() || signal_pending_interruption(NULL); + if (interrupted) { nready = -1; errno = EINTR; break; } + + /* An indefinite wait re-arms on a 200ms slice; break out when a master + * hung up during one, since kqueue will never make that fd ready. + */ + if (nready == 0 && !has_timeout) { + hup_ready = + epoll_collect_hung_up(inst, &hup_probe, &hup_probe_gen, 1) > 0; + if (hup_ready) + break; + } } while (nready == 0 && !has_timeout); int saved_errno = errno; - /* Remove wakeup pipe registration and drain if it fired */ - if (added_wakeup) { - struct kevent del_ev; - EV_SET(&del_ev, wakeup_pipe_rd, EVFILT_READ, EV_DELETE, 0, 0, NULL); - kevent(epoll_ref.fd, &del_ev, 1, NULL, 0, NULL); - wakeup_pipe_drain(); - /* Filter out wakeup pipe events from results */ - for (int i = 0; i < nready; i++) { - if ((uintptr_t) kevents[i].udata == (uintptr_t) -1) { - kevents[i] = kevents[nready - 1]; - nready--; - i--; - } - } - if (nready == 0 && !has_timeout) - goto epoll_retry; - } - /* Restore original signal mask after the blocking wait */ if (mask_installed) signal_restore_blocked(saved_mask); @@ -1376,6 +1499,13 @@ epoll_retry:; memset(out_index, 0xff, sizeof(out_index)); + /* Gather hung-up masters before taking inst->lock: the collector takes that + * lock itself, and it is not recursive. + */ + int hup_gfds[256]; + uint64_t hup_gens[256]; + int nhup = epoll_collect_hung_up(inst, hup_gfds, hup_gens, maxevents); + /* Serialize the regs[] reads and the oneshot re-arm against a concurrent * epoll_ctl or the close hook. Held only for this bookkeeping, never across * the blocking kevent() above; out[] is a local snapshot, so the guest @@ -1386,8 +1516,13 @@ epoll_retry:; for (int i = 0; i < nready && nout < maxevents; i++) { int gfd = (int) (uintptr_t) kevents[i].udata; - if (!RANGE_CHECK(gfd, 0, FD_TABLE_SIZE) || !inst->regs[gfd].active) + if (!RANGE_CHECK(gfd, 0, FD_TABLE_SIZE) || !inst->regs[gfd].active) { + struct kevent del; + EV_SET(&del, kevents[i].ident, kevents[i].filter, EV_DELETE, 0, 0, + NULL); + kevent(epoll_ref.fd, &del, 1, NULL, 0, NULL); continue; + } /* EPOLLONESHOT semantics: once any event fired and was reported, the fd * stays disarmed until EPOLL_CTL_MOD re-arms it. With multi-filter @@ -1415,6 +1550,39 @@ epoll_retry:; epoll_merge_event(&out[idx], &kevents[i], reg); } + /* Stamp EPOLLHUP for the masters the host cannot report on. Linux delivers + * EPOLLHUP whether or not the caller asked for it, so this ignores + * reg->events. Merging into an existing entry keeps a hangup that lands + * alongside queued output reported as EPOLLIN | EPOLLHUP, the way Linux + * does it -- the reader drains the shell's parting output before acting on + * the hangup. Runs before the oneshot marking below so an fd that fired + * this round still carries the hangup into that same report. + */ + for (int i = 0; i < nhup; i++) { + int gfd = hup_gfds[i]; + /* Re-check under the lock: the collector tested unlocked, so a + * concurrent epoll_ctl or close hook may have retired the entry since. + * The generation match is what rejects a DEL + re-ADD of the same fd + * number in that window, which would otherwise report this hangup + * against the new registration's epoll_data. + */ + if (!inst->regs[gfd].active || inst->regs[gfd].oneshot_armed || + inst->regs[gfd].generation != hup_gens[i]) + continue; + int idx = out_index[gfd]; + if (idx < 0) { + if (nout >= maxevents) + break; + idx = nout++; + out_index[gfd] = idx; + out_gfds[idx] = gfd; + out[idx].events = 0; + out[idx]._pad = 0; + out[idx].data = inst->regs[gfd].data; + } + out[idx].events |= LINUX_EPOLLHUP; + } + /* Mark EPOLLONESHOT FDs as armed (fired but waiting for MOD re-arm). kqueue * already removed the event (EV_ONESHOT), so poll emulation marks the * registration as oneshot_armed to allow MOD but prevent further event @@ -1432,12 +1600,13 @@ epoll_retry:; /* Write results to guest */ if (nout > 0) { - if (guest_write_small(g, events_gva, out, - nout * sizeof(linux_epoll_event_t)) < 0) { - host_fd_ref_close(&epoll_ref); - epoll_instance_release(inst); - return -LINUX_EFAULT; - } + for (int i = 0; i < nout; i++) + if (guest_write_small(g, events_gva, out, + nout * sizeof(linux_epoll_event_t)) < 0) { + host_fd_ref_close(&epoll_ref); + epoll_instance_release(inst); + return -LINUX_EFAULT; + } } host_fd_ref_close(&epoll_ref); diff --git a/src/syscall/poll.h b/src/syscall/poll.h index 37901e6f..11c0b948 100644 --- a/src/syscall/poll.h +++ b/src/syscall/poll.h @@ -41,7 +41,7 @@ int64_t sys_epoll_pwait(guest_t *g, * Called from the fd-table close chokepoint; caller holds fd_lock (or runs * single-threaded on the relaxed close path). */ -void epoll_note_fd_closed(int closed_fd); +void epoll_note_fd_closed(int closed_fd, uint64_t closed_ofd_id); /* Free an epoll instance (fd_table[epfd].dir) and drop it from the live count. * Called from fd_cleanup_entry() for FD_EPOLL slots. diff --git a/tests/test-pty.c b/tests/test-pty.c index 3d808723..8834aa25 100644 --- a/tests/test-pty.c +++ b/tests/test-pty.c @@ -32,14 +32,17 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include +#include #include #include "test-harness.h" @@ -794,6 +797,382 @@ int main(void) } } + /* The same hangup through epoll. poll and epoll answer from the same pty + * bookkeeping, but they are separate readiness paths: epoll_wait needs its + * own wiring, and a terminal that waits with epoll (foot does) hangs on + * exit without it. + */ + { + int ep_master = open("/dev/ptmx", O_RDWR | O_NOCTTY); + unsigned int ep_ptyno = (unsigned int) -1; + int ep_unlock = 0; + if (ep_master < 0 || ioctl(ep_master, TIOCGPTN, &ep_ptyno) != 0 || + ioctl(ep_master, TIOCSPTLCK, &ep_unlock) != 0) { + TEST("epoll reports EPOLLHUP once the slave closes"); + FAIL("could not stage a master"); + if (ep_master >= 0) + close(ep_master); + } else { + char ep_path[32]; + snprintf(ep_path, sizeof(ep_path), "/dev/pts/%u", ep_ptyno); + int ep_slave = open(ep_path, O_RDWR | O_NOCTTY); + int ep = epoll_create1(0); + if (ep_slave < 0 || ep < 0) { + TEST("epoll reports EPOLLHUP once the slave closes"); + FAIL("could not stage slave/epoll fd"); + if (ep_slave >= 0) + close(ep_slave); + } else { + close(ep_slave); + + struct epoll_event reg = {.events = EPOLLIN}; + reg.data.u64 = 0x5eed; + int added = epoll_ctl(ep, EPOLL_CTL_ADD, ep_master, ®); + + /* A finite timeout: before the fix this returned 0 here and + * blocked forever on the -1 an actual terminal passes. + */ + struct epoll_event ev; + memset(&ev, 0, sizeof(ev)); + int er = added == 0 ? epoll_wait(ep, &ev, 1, 2000) : -1; + + TEST("epoll reports EPOLLHUP once the slave closes"); + EXPECT_TRUE(er > 0 && (ev.events & EPOLLHUP), + "no EPOLLHUP after the last slave closed"); + + TEST("EPOLLHUP carries the registered epoll_data"); + EXPECT_TRUE(er > 0 && ev.data.u64 == 0x5eed, + "hangup event lost its user data"); + + /* An already-pending hangup must not be held until the + * caller's deadline: the host never makes the fd ready, so a + * finite wait that only checks after kevent returns would + * block for the full timeout before reporting. + */ + struct timespec t0, t1; + clock_gettime(CLOCK_MONOTONIC, &t0); + memset(&ev, 0, sizeof(ev)); + int er2 = epoll_wait(ep, &ev, 1, 10000); + clock_gettime(CLOCK_MONOTONIC, &t1); + long elapsed_ms = (t1.tv_sec - t0.tv_sec) * 1000 + + (t1.tv_nsec - t0.tv_nsec) / 1000000; + + /* The elapsed check is what actually detects the bug and cannot + * be dropped: the unfixed path still delivers the event, just + * after sitting out the whole timeout, so er2 and EPOLLHUP look + * identical either way. The two outcomes are ~0ms and ~10000ms, + * so the threshold sits halfway between them -- far enough from + * both that scheduling noise on a loaded host cannot reach it. + */ + TEST("a pending EPOLLHUP does not wait out a finite timeout"); + EXPECT_TRUE( + er2 > 0 && (ev.events & EPOLLHUP) && elapsed_ms < 5000, + "hangup was delayed until the epoll_wait deadline"); + } + if (ep >= 0) + close(ep); + close(ep_master); + } + } + + /* A slave obtained through TIOCGPTPEER has to count toward the same + * accounting as a /dev/pts/N open. It has been the recommended way to reach + * the peer since Linux 4.13, and a master whose only slave arrived that way + * would otherwise never report a hangup at all. + */ + { + int gp_master = open("/dev/ptmx", O_RDWR | O_NOCTTY); + int gp_unlock = 0; + unsigned int gp_ptyno = (unsigned int) -1; + if (gp_master < 0 || ioctl(gp_master, TIOCGPTN, &gp_ptyno) != 0 || + ioctl(gp_master, TIOCSPTLCK, &gp_unlock) != 0) { + TEST("TIOCGPTPEER slave counts toward the hangup"); + FAIL("could not stage a master"); + if (gp_master >= 0) + close(gp_master); + } else { + int gp_slave = ioctl(gp_master, TIOCGPTPEER, O_RDWR | O_NOCTTY); + if (gp_slave < 0) { + /* Matches the existing TIOCGPTPEER case above, which accepts + * ENOTTY on hosts without peer support. + */ + TEST("TIOCGPTPEER slave counts toward the hangup"); + EXPECT_TRUE( + errno == ENOTTY, + "TIOCGPTPEER failed for a reason other than ENOTTY"); + } else { + close(gp_slave); + + struct pollfd gp = {.fd = gp_master, .events = POLLIN}; + int gr = poll(&gp, 1, 2000); + TEST("TIOCGPTPEER slave counts toward the hangup"); + EXPECT_TRUE(gr > 0 && (gp.revents & POLLHUP), + "no POLLHUP after the TIOCGPTPEER slave closed"); + } + close(gp_master); + } + } + + /* The shape a real terminal has: the master stays in this process and the + * slave lives in a forked child, which is the shell. "exit" in the terminal + * is that child going away. + * + * A guest fork is a posix_spawn of a fresh elfuse process, so the child + * keeps its own keepalive table and nothing about its slave reaches the + * parent's accounting by itself. Without the shared per-pty counters this + * reports no hangup on either readiness path, and foot's window stays open + * after the shell exits even though the same-process cases above pass. + */ + { + int fk_master = open("/dev/ptmx", O_RDWR | O_NOCTTY); + unsigned int fk_ptyno = (unsigned int) -1; + int fk_unlock = 0; + if (fk_master < 0 || ioctl(fk_master, TIOCGPTN, &fk_ptyno) != 0 || + ioctl(fk_master, TIOCSPTLCK, &fk_unlock) != 0) { + TEST("master reports POLLHUP when a forked child's slave closes"); + FAIL("could not stage a master"); + if (fk_master >= 0) + close(fk_master); + } else { + char fk_path[32]; + snprintf(fk_path, sizeof(fk_path), "/dev/pts/%u", fk_ptyno); + + pid_t fk_pid = fork(); + if (fk_pid == 0) { + /* The shell: take the slave, say something, leave. */ + int slave = open(fk_path, O_RDWR | O_NOCTTY); + if (slave < 0) + _exit(3); + write(slave, "bye", 3); + close(slave); + _exit(0); + } + + int fk_status = 0; + bool reaped = + fk_pid > 0 && waitpid(fk_pid, &fk_status, 0) == fk_pid; + bool child_ok = + reaped && WIFEXITED(fk_status) && WEXITSTATUS(fk_status) == 0; + + if (fk_pid < 0) { + /* Distinct from a child-side failure: nothing ran, so this says + * nothing about the pty. Reporting it as a slave-open failure + * would send the next reader after the wrong bug. + */ + TEST( + "master reports POLLHUP when a forked child's slave " + "closes"); + FAIL("fork failed, no child to hold the slave"); + } else if (!child_ok) { + TEST( + "master reports POLLHUP when a forked child's slave " + "closes"); + FAIL("child could not open the slave"); + } else { + /* Queued output still comes first, exactly as in the + * same-process case: drain before judging the hangup. + */ + char fk_buf[32]; + struct pollfd fk_drain = {.fd = fk_master, .events = POLLIN}; + if (poll(&fk_drain, 1, 1000) > 0 && (fk_drain.revents & POLLIN)) + read(fk_master, fk_buf, sizeof(fk_buf)); + + struct pollfd fp = {.fd = fk_master, .events = POLLIN}; + int fr = poll(&fp, 1, 2000); + TEST( + "master reports POLLHUP when a forked child's slave " + "closes"); + EXPECT_TRUE(fr > 0 && (fp.revents & POLLHUP), + "no POLLHUP after the forked child's slave closed"); + + int fep = epoll_create1(0); + struct epoll_event freg = {.events = EPOLLIN}; + freg.data.u64 = 0xf00d; + struct epoll_event fev; + memset(&fev, 0, sizeof(fev)); + int fer = -1; + if (fep >= 0 && + epoll_ctl(fep, EPOLL_CTL_ADD, fk_master, &freg) == 0) + fer = epoll_wait(fep, &fev, 1, 2000); + TEST( + "epoll reports EPOLLHUP when a forked child's slave " + "closes"); + EXPECT_TRUE(fer > 0 && (fev.events & EPOLLHUP), + "no EPOLLHUP after the forked child's slave " + "closed"); + if (fep >= 0) + close(fep); + } + close(fk_master); + } + } + + /* The exit path a real shell takes: the child never closes its slave, it + * just exits and lets the kernel reap the fds. Nothing passes through the + * per-fd close hook, so the accounting has to be settled at process + * teardown instead. Until it was, foot started fine and then sat there with + * its window open after "exit" -- the same-process and explicit-close cases + * above both passed the whole time, which is why this needs its own case. + */ + { + int ex_master = open("/dev/ptmx", O_RDWR | O_NOCTTY); + unsigned int ex_ptyno = (unsigned int) -1; + int ex_unlock = 0; + if (ex_master < 0 || ioctl(ex_master, TIOCGPTN, &ex_ptyno) != 0 || + ioctl(ex_master, TIOCSPTLCK, &ex_unlock) != 0) { + TEST("hangup when a child exits still holding its slave"); + FAIL("could not stage a master"); + if (ex_master >= 0) + close(ex_master); + } else { + char ex_path[32]; + snprintf(ex_path, sizeof(ex_path), "/dev/pts/%u", ex_ptyno); + + pid_t ex_pid = fork(); + if (ex_pid == 0) { + /* Deliberately no close(): exit with the slave still open. */ + int slave = open(ex_path, O_RDWR | O_NOCTTY); + if (slave < 0) + _exit(3); + _exit(0); + } + + int ex_status = 0; + bool ex_reaped = + ex_pid > 0 && waitpid(ex_pid, &ex_status, 0) == ex_pid; + bool ex_ok = ex_reaped && WIFEXITED(ex_status) && + WEXITSTATUS(ex_status) == 0; + + if (ex_pid < 0) { + TEST("hangup when a child exits still holding its slave"); + FAIL("fork failed, no child to hold the slave"); + } else if (!ex_ok) { + TEST("hangup when a child exits still holding its slave"); + FAIL("child could not open the slave"); + } else { + struct pollfd xp = {.fd = ex_master, .events = POLLIN}; + int xr = poll(&xp, 1, 2000); + TEST("hangup when a child exits still holding its slave"); + EXPECT_TRUE(xr > 0 && (xp.revents & POLLHUP), + "no POLLHUP after the child exited holding it"); + + int xep = epoll_create1(0); + struct epoll_event xreg = {.events = EPOLLIN}; + struct epoll_event xev; + memset(&xev, 0, sizeof(xev)); + int xer = -1; + if (xep >= 0 && + epoll_ctl(xep, EPOLL_CTL_ADD, ex_master, &xreg) == 0) + xer = epoll_wait(xep, &xev, 1, 2000); + TEST("epoll hangup when a child exits still holding its slave"); + EXPECT_TRUE(xer > 0 && (xev.events & EPOLLHUP), + "no EPOLLHUP after the child exited holding it"); + if (xep >= 0) + close(xep); + } + close(ex_master); + } + } + + /* What a terminal actually does to hand a shell its tty: dup2 the slave + * onto stdin/stdout/stderr, then close the original fd. Only the open() was + * ever counted, so those three live references were invisible and the + * close of the original drove the count to zero -- the master reported a + * hangup with the shell still running. Every other case here opens a slave + * and keeps that same fd, which is why none of them caught it. + */ + { + int dp_master = open("/dev/ptmx", O_RDWR | O_NOCTTY); + unsigned int dp_ptyno = (unsigned int) -1; + int dp_unlock = 0; + if (dp_master < 0 || ioctl(dp_master, TIOCGPTN, &dp_ptyno) != 0 || + ioctl(dp_master, TIOCSPTLCK, &dp_unlock) != 0) { + TEST("dup2'd slave keeps the master from hanging up"); + FAIL("could not stage a master"); + if (dp_master >= 0) + close(dp_master); + } else { + char dp_path[32]; + snprintf(dp_path, sizeof(dp_path), "/dev/pts/%u", dp_ptyno); + int dp_slave = open(dp_path, O_RDWR | O_NOCTTY); + int dp_alias = dp_slave >= 0 ? dup(dp_slave) : -1; + if (dp_slave < 0 || dp_alias < 0) { + TEST("dup2'd slave keeps the master from hanging up"); + FAIL("could not stage slave/dup"); + if (dp_slave >= 0) + close(dp_slave); + } else { + /* Drop the original; only the dup still references the pty. */ + close(dp_slave); + + struct pollfd dp = {.fd = dp_master, .events = POLLIN}; + int dr = poll(&dp, 1, 300); + TEST("dup2'd slave keeps the master from hanging up"); + EXPECT_TRUE(!(dr > 0 && (dp.revents & POLLHUP)), + "master hung up while a dup of the slave was open"); + + /* And the hangup still arrives once the dup goes too. */ + close(dp_alias); + struct pollfd dp2 = {.fd = dp_master, .events = POLLIN}; + int dr2 = poll(&dp2, 1, 2000); + TEST("hangup arrives once the dup'd slave closes"); + EXPECT_TRUE(dr2 > 0 && (dp2.revents & POLLHUP), + "no POLLHUP after the last dup closed"); + } + close(dp_master); + } + } + + /* readv must report the hangup too, not just read. + * + * Only read(2) used to answer EIO, so a terminal that drains its master + * with readv got the POLLHUP, called readv, and blocked there forever with + * its window still open -- the host never fails the read, because elfuse's + * keepalive slave keeps the pty alive from its point of view. This is what + * foot does, and no amount of correct hangup accounting helped while the + * read it actually uses never returned. + */ + { + int rv_master = open("/dev/ptmx", O_RDWR | O_NOCTTY); + unsigned int rv_ptyno = (unsigned int) -1; + int rv_unlock = 0; + if (rv_master < 0 || ioctl(rv_master, TIOCGPTN, &rv_ptyno) != 0 || + ioctl(rv_master, TIOCSPTLCK, &rv_unlock) != 0) { + TEST("readv reports the hangup as EIO"); + FAIL("could not stage a master"); + if (rv_master >= 0) + close(rv_master); + } else { + char rv_path[32]; + snprintf(rv_path, sizeof(rv_path), "/dev/pts/%u", rv_ptyno); + int rv_slave = open(rv_path, O_RDWR | O_NOCTTY); + if (rv_slave < 0) { + TEST("readv reports the hangup as EIO"); + FAIL("open slave"); + } else { + close(rv_slave); + + /* Guard the whole thing: before the fix readv never returned, + * so a regression wedges the suite rather than failing it. + */ + signal(SIGALRM, hup_on_alarm); + alarm(10); + + char rb1[16], rb2[16]; + struct iovec riov[2] = {{rb1, sizeof(rb1)}, {rb2, sizeof(rb2)}}; + errno = 0; + ssize_t rvret = readv(rv_master, riov, 2); + int rverr = errno; + alarm(0); + + TEST("readv reports the hangup as EIO"); + EXPECT_TRUE(rvret < 0 && rverr == EIO, + "readv did not report the hangup as EIO"); + } + close(rv_master); + } + } + SUMMARY("test-pty"); return fails > 0 ? 1 : 0; }