Skip to content

fix: close leaked fds in openpty fallback, add static assertions#24

Open
rmorgans wants to merge 3 commits intomobydeck:mainfrom
rmorgans:fix/openpty-and-asserts
Open

fix: close leaked fds in openpty fallback, add static assertions#24
rmorgans wants to merge 3 commits intomobydeck:mainfrom
rmorgans:fix/openpty-and-asserts

Conversation

@rmorgans
Copy link

Summary

  1. openpty fallback fd leak: the fallback openpty() in master.c (used on platforms without <pty.h>) opens /dev/ptmx but leaks the fd if grantpt(), unlockpt(), or slave open() fails. Fixed with a single goto fail cleanup path.
  2. _Static_assert for SCROLLBACK_SIZE: the ring buffer uses & (SCROLLBACK_SIZE - 1) for index wrapping, which requires a positive power of two. Now enforced at compile time. SCROLLBACK_SIZE == 0 is also rejected.
  3. _Static_assert for pkt.u.buf: struct packet uses unsigned char len — the buffer must fit in 255 bytes. Now enforced at compile time.

What bugs these fix

  • fd leak: on platforms using the fallback openpty, repeated pty allocation failures would exhaust the file descriptor table.
  • ring buffer: misconfiguring SCROLLBACK_SIZE to a non-power-of-two or zero would silently corrupt the scrollback buffer. Now caught at compile time.
  • packet length: if struct winsize ever exceeded 255 bytes on a platform, pkt.len would silently truncate.

Test results

202/203 pass (1 pre-existing: test 93). ASan+UBSan clean.

rmorgans and others added 2 commits March 14, 2026 22:48
- 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
rmorgans added a commit to rmorgans/atch that referenced this pull request Mar 15, 2026
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) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant