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; }