From 988d54bcd1e6ccdd721e6422bc9085fcac315263 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sat, 14 Mar 2026 22:48:16 +1030 Subject: [PATCH 1/3] fix: close leaked fds in openpty fallback, add static assertions - openpty fallback now closes master fd on grantpt/unlockpt/ptsname failure instead of leaking it - Add _Static_assert for SCROLLBACK_SIZE power-of-two requirement and packet buffer fitting in uint8_t length field Co-Authored-By: Claude Opus 4.6 (1M context) --- atch.h | 2 ++ config.h | 2 ++ master.c | 21 ++++++++++++++------- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/atch.h b/atch.h index 0b59f48..e096256 100644 --- a/atch.h +++ b/atch.h @@ -115,6 +115,8 @@ struct packet struct winsize ws; } u; }; +_Static_assert(sizeof(((struct packet *)0)->u.buf) <= 255, + "packet buffer must fit in uint8_t length"); /* ** The master sends a simple stream of text to the attaching clients, without diff --git a/config.h b/config.h index 398c905..3b70f94 100644 --- a/config.h +++ b/config.h @@ -22,6 +22,8 @@ #ifndef SCROLLBACK_SIZE #define SCROLLBACK_SIZE (128 * 1024) #endif +_Static_assert((SCROLLBACK_SIZE & (SCROLLBACK_SIZE - 1)) == 0, + "SCROLLBACK_SIZE must be a power of two"); /* Maximum size of the on-disk session log; older bytes are trimmed on open */ #ifndef LOG_MAX_SIZE diff --git a/master.c b/master.c index 16f29e3..164196e 100644 --- a/master.c +++ b/master.c @@ -802,29 +802,29 @@ int openpty(int *amaster, int *aslave, char *name, struct termios *termp, struct winsize *winp) { - int master, slave; + int master = -1, slave = -1; char *buf; master = open("/dev/ptmx", O_RDWR); if (master < 0) return -1; if (grantpt(master) < 0) - return -1; + goto fail; if (unlockpt(master) < 0) - return -1; + goto fail; buf = ptsname(master); if (!buf) - return -1; + goto fail; slave = open(buf, O_RDWR | O_NOCTTY); if (slave < 0) - return -1; + goto fail; #ifdef I_PUSH if (ioctl(slave, I_PUSH, "ptem") < 0) - return -1; + goto fail; if (ioctl(slave, I_PUSH, "ldterm") < 0) - return -1; + goto fail; #endif *amaster = master; @@ -836,6 +836,13 @@ openpty(int *amaster, int *aslave, char *name, struct termios *termp, if (winp) ioctl(slave, TIOCSWINSZ, winp); return 0; + +fail: + if (master >= 0) + close(master); + if (slave >= 0) + close(slave); + return -1; } pid_t From adc1181ca7b42a4c914e22a62bcbbd9ffd425996 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sat, 14 Mar 2026 23:26:23 +1030 Subject: [PATCH 2/3] fix: reject SCROLLBACK_SIZE == 0 in static assertion Zero passes the power-of-two bitmask check but is not a valid ring buffer size. Add SCROLLBACK_SIZE > 0 guard. Co-Authored-By: Claude Opus 4.6 (1M context) --- config.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config.h b/config.h index 3b70f94..1e3dfcb 100644 --- a/config.h +++ b/config.h @@ -22,8 +22,8 @@ #ifndef SCROLLBACK_SIZE #define SCROLLBACK_SIZE (128 * 1024) #endif -_Static_assert((SCROLLBACK_SIZE & (SCROLLBACK_SIZE - 1)) == 0, - "SCROLLBACK_SIZE must be a power of two"); +_Static_assert(SCROLLBACK_SIZE > 0 && (SCROLLBACK_SIZE & (SCROLLBACK_SIZE - 1)) == 0, + "SCROLLBACK_SIZE must be a positive power of two"); /* Maximum size of the on-disk session log; older bytes are trimmed on open */ #ifndef LOG_MAX_SIZE From 329bbefddf7531601529acae74470ac9af388ebb Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sun, 15 Mar 2026 11:50:10 +1030 Subject: [PATCH 3/3] test: add fd leak regression test for openpty fallback Cycles 50 sessions under ulimit -n 64 to verify that the openpty fallback properly closes fds on failure paths. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test.sh | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test.sh b/tests/test.sh index 0c954c0..400e078 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -720,6 +720,34 @@ assert_contains "no args: shows Usage:" "Usage:" "$out" run "$ATCH" --help assert_contains "help: shows tail command" "tail" "$out" +# ── 23. fd leak: rapid session cycling under low fd limit ────────────────── +# openpty fallback leaks fds on error paths. Under a tight fd limit, +# leaked fds accumulate and eventually prevent new sessions from starting. + +( + ulimit -n 64 2>/dev/null || true + LEAK_FAIL=0 + i=0 + while [ $i -lt 50 ]; do + out=$("$ATCH" start "leak-$i" sleep 999 2>&1) + lrc=$? + if [ "$lrc" -ne 0 ]; then + LEAK_FAIL=1 + break + fi + "$ATCH" kill "leak-$i" >/dev/null 2>&1 + sleep 0.02 + i=$((i + 1)) + done + i=0; while [ $i -lt 50 ]; do "$ATCH" kill "leak-$i" >/dev/null 2>&1; i=$((i + 1)); done + exit $LEAK_FAIL +) +if [ $? -eq 0 ]; then + ok "fd-leak: 50 create/destroy cycles under ulimit -n 64" +else + fail "fd-leak: session failed under low fd limit (possible fd leak)" "50 cycles" "failed early" +fi + # ── summary ────────────────────────────────────────────────────────────────── printf "\n1..%d\n" "$T"