feat(tty): Implement missing TTY termios ioctls (TCSETSF, TCSETA*)#2141
Merged
fslongjin merged 37 commits intoJul 23, 2026
Merged
Conversation
added 8 commits
July 17, 2026 23:16
Add legacy SVR4 struct termio representation (NCC=8) with from_kernel_termios() and to_kernel_termios() conversions. Speed is extracted from c_cflag CBAUD/CIBAUD bits, matching the existing PosixTermios::to_kernel_termios() pattern. This is the foundation for wiring TCGETA/TCSETA/TCSETAW/TCSETAF.
Add a drain_output() method that attempts to flush pending ldisc output (opost + echo buffers) without blocking indefinitely. Returns Ok(true) on full drain, Ok(false) when output_lock is held by a concurrent writer. Default trait impl is a no-op returning Ok(true); NTty overrides with try_lock-based drain of opost_pending and echo buffers. This is the primitive used by the upcoming TERMIOS_WAIT drain.
Replace todo!() with real termio → termios conversion. Conversion semantics follow Linux 6.6 generic user_termio_to_kernel_termios(): low 16 bits of each flag word come from the user termio, high 16 bits are preserved from the current termios; only the first NCC c_cc slots are overwritten. Speeds are derived from the merged c_cflag after conversion, mirroring how Linux set_termios() calls tty_termios_baud_rate() after user_termio_to_kernel_termios().
Map the legacy termio ioctl family onto core_set_termios with the TERMIOS_TERMIO flag, plus TERMIOS_WAIT/TERMIOS_FLUSH for the drain/flush variants. TCGETA serializes the current termios through PosixTermio::from_kernel_termios. Flag mapping matches Linux 6.6 tty_ioctl.c:838-845.
Replace the // TODO with a best-effort output drain: 1. drain_output() flushes ldisc opost + echo buffers. 2. A bounded loop (100 iterations) polls chars_in_buffer() with sched_yield() between iterations, giving device IRQs CPU time to drain hardware FIFOs. On PTY the loop exits immediately. This mirrors the intent of Linux wait_until_sent: output pending at the time of TCSETSW/TCSETSF/TCSETAW/TCSETAF is transmitted before the new termios takes effect. A full blocking tty_wait_until_sent with wait-queue timeout is future work.
Covers the dpkg/apt regression: tcsetattr(TCSAFLUSH) on a valid PTY slave, plus legacy TCGETA/TCSETA/TCSETAW/TCSETAF ioctls. Also verifies termio merge semantics (high 16 bits of c_cflag preserved) and ENOTTY on non-TTY fds.
LineDisciplineType::from_line() previously panicked on non-zero input via todo!(). Unknown/unsupported line disciplines now fall back to NTty, matching Linux behaviour where N_TTY is the default when a line discipline is not registered. This is a P0 fix: a malicious termio with c_line=255 would crash the kernel. The PosixTermios path has the same latent bug but the termio path adds new user-facing surface area.
DragonOS /dev/null returns ENOSYS (38) for TTY ioctls, not ENOTTY. Accept any non-zero errno on tcsetattr failure against a non-TTY fd — the point is verifying we don't crash or succeed, not enforcing the exact error code.
Member
|
@codex review |
4 tasks
Port the c_unitest/test_tty_termios.c coverage into Google Test format under dunitest/suites/normal/ so it runs in CI (make test-dunit). Covers: TCSANOW round-trip, TCSADRAIN, TCSAFLUSH (dpkg regression), legacy TCGETA/TCSETA/TCSETAW/TCSETAF, low-16-bit cross-check, merge semantics, non-TTY fd rejection.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce0ffaa48d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
added 10 commits
July 18, 2026 00:45
…ack leak P1: PosixTermio has 17 bytes of fields. repr(C) pads to 18 bytes (u16 alignment), leaving 1 byte of implicit tail padding. TCGETA copies the full 18 bytes to userspace via copy_one_to_user, leaking a stack byte. Add an explicit pub _pad: u8 field after c_cc. Default::default() zeroes it, and from_kernel_termios sets it to 0 explicitly.
P1 fixes for TERMIOS_WAIT: 1. drain_output() return value was silently discarded via . When output_lock is held by a concurrent writer, drain_output returns Ok(false) without inspecting opost state. Now retry up to 8 times with sched_yield() between attempts. 2. Replace fixed 100-iteration sched_yield spin-loop with event-driven wait on write_wq (wait_event_interruptible on EPOLLOUT|EPOLLWRNORM). The kernel wakes waiters when the hardware TX completes, avoiding the race where slow serial links (e.g. 9600 baud, 4KB output) would exhaust the fixed retry budget before the FIFO actually drained.
P2: TCSETA/TCSETAW/TCSETAF entered core_set_termios without tty_check_change, allowing background process groups to modify terminal attributes without receiving SIGTTOU. Linux 6.6 calls tty_check_change() at the start of set_termios() — add the same guard here, covering both termios and legacy termio setter paths. Also covers TCSETS/TCSETSW/TCSETSF which were already in tty_mode_ioctl but also bypassed the check (same code path).
P2: PosixTermios::to_kernel_termios() mapped non-zero c_line to NTty via LineDisciplineType::from_line(), and from_kernel_termios() wrote back (always 0). Users setting c_line via TCSETS/TCSETA lost the value on TCGETS/TCGETA — a round-trip corruption. Add c_line_abi: u8 to Termios to store the raw ABI byte. All conversions (PosixTermios, PosixTermio, TTY_STD_TERMIOS, TTY_SERIAL_DEFAULT_TERMIOS) now round-trip c_line_abi unchanged, matching Linux which treats c_line as an opaque byte and switches line disciplines via TIOCSETD, not via c_line in termios.
H1: Extract CIBAUD_SHIFT=16 constant and ControlMode::input_baud_rate() method. PosixTermios::input_speed() and PosixTermio::speeds_from_cflag() now both delegate to the shared implementation, eliminating the hard-coded >> 16 duplication. M3: Change PosixTermio::_pad from pub to pub(crate) to prevent external code from bypassing the zero-init leak protection. M4: Unify from_kernel_termios signatures — both PosixTermios and PosixTermio now take &Termios (by reference). L6: Extract INIT_C_LINE_ABI constant shared by TTY_STD_TERMIOS and TTY_SERIAL_DEFAULT_TERMIOS.
M1: Increase drain_output retry from 8 to 32 iterations with TODO
for wait_event-based output_lock release tracking.
M2: Document why wait_event_interruptible return value is ignored
(matching Linux tty_wait_until_sent behaviour). Add TODO for
timeout protection in hung-hardware scenarios.
L1: Replace silent with log::debug! when
drain_output exhausts all retries without success.
L2: Add log::debug! in LineDisciplineType::from_line for non-zero
c_line values falling back to NTty.
… ioctl L4: CHECK(errno != ENOTTY) and EXPECT_NE(errno, ENOTTY) guards after a successful ioctl call are meaningless — errno is undefined on success. The ioctl == 0 check already confirms the call did not fail, making the errno check redundant. Removed from both c_unitest and dunitest versions.
L3: Add header comment in test_tty_termios.c explaining why both the
C (c_unitest) and C++ (dunitest) versions exist — the C version is
a fast self-contained smoke test for environments where dunitest
cannot run (no gtest/runner/rootfs).
L5: Add TODO comments in both test files noting the missing drain
stress test (master writes data → slave TCSADRAIN → verify drain
actually waited for output completion).
C1: B3500000 (0x0000100e) and B4000000 (0x0000100f) were accidentally dropped from ControlMode::baud_rate() during the v1→v2 dedup refactor. Restore them — both speeds are defined in the bitflags and covered by the CBAUD mask, so baud_rate() must handle them or the constants should be removed (the former is correct). L1: Move CIBAUD_SHIFT from between bitflags! and impl ControlMode to the constant area near NCC / INIT_C_LINE_ABI. L2: Merge duplicate header comment block in test_tty_termios.c into a single clean file-level doc.
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e4e539dc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kaleidoscope416
marked this pull request as draft
July 18, 2026 14:20
kaleidoscope416
force-pushed
the
feat/tty-termios-toctls
branch
2 times, most recently
from
July 18, 2026 14:54
e0f7e35 to
1425432
Compare
Squash of 27 review-driven fixes across 5 review documents covering: - TCSADRAIN drain loop with hardware FIFO wait (F1, DragonOS-Community#1) - drain_echoes best-effort + Ok(false) logging (F3, M2, DragonOS-Community#4) - drain_output trait doc contract correction (F2, F3, DragonOS-Community#2, DragonOS-Community#3) - _pad stack-leak prevention doc + compile-time sizeof assert (H1, P3-1) - test struct _pad ABI match (F2) - speed derivation moved into to_kernel_termios (F5) - from_line log level + maintenance doc (H2, P3-2) - PTY drain loop docs (M1) - tty_check_change path docs (M3) - inline CIBAUD_SHIFT (L1) - remove unused name[256] (L2) - test: uninitialized struct fix (P3-3) - test: CHECK_NOERR macro (P3-4) - test: ECHO flip no-op fix (F5) - test: errno-before-close fix (F6) - test: /dev/null skip message (DragonOS-Community#7) - test: c_line round-trip (DragonOS-Community#8) - test: dead code removal (DragonOS-Community#10)
kaleidoscope416
force-pushed
the
feat/tty-termios-toctls
branch
from
July 18, 2026 14:55
1425432 to
c3eb3f4
Compare
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f9754e615
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep PTY write readiness tied to available bridge queue room while retaining queued-byte accounting for TCSADRAIN. This restores POLLOUT as soon as peer reads release capacity without changing serial wakeup thresholds. Preserve the original PTY master ioctl context when termios operations are redirected to the slave. A closed slave output direction is no longer treated as EIO or waited on, matching Linux while still applying flush and attribute updates. Add regression coverage for master drain actions after a saturated slave closes, and make the existing drain waiter startup failure path join-safe. Tested with: make fmt; make all; tty_termios_test (13/13); cubesandbox_pty_exec_chain_test (27/27); tty_pty_hangup_test (33/33); spawn_exec_pipe_race_test (1/1). Signed-off-by: longjin <longjin@dragonos.org>
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ffb187fbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Treat the DragonOS PTY bridge as peer input buffering, matching the Linux PTY flip-buffer ownership model. Keep bridge capacity in write_room and POLLOUT, but leave chars_in_buffer at the PTY default so TCSADRAIN never depends on peer reads. Preserve waits for output still retained by N_TTY. Track hangup generations per open tty file to model Linux hung_up_tty_fops without conflating driver IO_ERROR or breaking later reopens. Existing slave fds now return EIO for mode ioctls after master hangup, with the Linux TIOCSPGRP ENOTTY exception, while PTY master redirection remains usable. Add regression coverage for unread PTY input, retained echo ownership, and modern and legacy mode ioctls on hung-up slaves. Tested with: make fmt; make kernel; tty_termios_test (14/14); tty_pty_hangup_test (33/33); cubesandbox_pty_exec_chain_test (27/27); focused drain and hangup tests repeated 5 times. Signed-off-by: longjin <longjin@dragonos.org>
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23dde82064
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep file descriptions opened before a hangup permanently disconnected even when the TTY is reopened. Apply Linux-compatible read, write, poll, epoll, ioctl, and fasync behavior through the per-file hangup generation. Propagate fasync handler errors before committing FASYNC while preserving the distinct unsupported-handler semantics of F_SETFL and FIOASYNC. Add exact PTY hangup regression coverage. Signed-off-by: longjin <longjin@dragonos.org>
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce5d1ab5e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep per-file hangup generations visible throughout N_TTY blocking operations so reopening a terminal cannot revive an operation from an older generation. Serialize 8250 readiness checks with FIFO writes, bound nested emergency output, and reset transmit FIFO state after a close-time timeout. Signed-off-by: longjin <longjin@dragonos.org>
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a887b32c4a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Use a fixed Linux-aligned 10 ms readiness deadline for emergency 8250 output so low baud rates cannot cause multi-second stalls with interrupts or preemption disabled. Signed-off-by: longjin <longjin@dragonos.org>
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9851f9ac72
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Invalidate runtime console queue snapshots across flushes so cleared data cannot leave the old-drain loop spinning or consume later writes. Make emergency output acquire the complete UART lock tuple with try-locks and restore IER state without any blocking lock acquisition. Signed-off-by: longjin <longjin@dragonos.org>
Let successful fasync handlers own the FASYNC status bit, leave unsupported F_SETFL requests uncommitted, and roll back other status flags when a handler transition fails. Add regression coverage for unsupported F_SETFL and FIOASYNC behavior. Signed-off-by: longjin <longjin@dragonos.org>
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e81a60c2b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Track TTY fasync registrations by open file description and clear them atomically when a hangup starts. Serialize status-flag updates with lifecycle cleanup so concurrent F_SETFL calls cannot restore stale FASYNC state, and unregister fasync entries during final file release. Route legacy 8250 ports through their Linux-compatible IRQ3/IRQ4 lines. Service only pending UARTs on the asserted shared IRQ, bound the rescan loop, preserve NotHandled semantics, and avoid exposing ports without an enumerated interrupt resource. Add PTY hangup and concurrent flag-update regressions; validate COM2 transmission independently from IRQ4. Signed-off-by: longjin <longjin@dragonos.org>
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 549c623bb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Clearing the 8250 software TX queue restores write readiness, but the flush path only woke blocking writers. Notify registered epoll instances with EPOLLOUT and EPOLLWRNORM so waiters cannot remain asleep without a later TX interrupt. Keep the notification separate from tty_wakeup because signal-character flushing may enter the driver while holding the N_TTY data lock; a synchronous line-discipline write_wakeup callback would re-enter that lock. Signed-off-by: longjin <longjin@dragonos.org>
Member
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Complete the Linux-compatible TTY termios ioctl path and harden the surrounding TTY, PTY, serial8250, hangup, and asynchronous-notification lifecycles uncovered during review.
The original user-visible failure was
tcsetattr(..., TCSAFLUSH, ...)returningENOTTY, which caused package configuration tools such asdebconfto fail:This PR now provides the missing ioctl support and the drain, flush, synchronization, hardware-transmit, hangup, and wakeup semantics required for those operations to behave correctly under concurrency.
User-visible behavior
TCGETA,TCSETA,TCSETAW, andTCSETAFusing the legacy SVR4struct termioABI.TCSETSW/TCSETAW(TCSADRAIN).TCSETSF/TCSETAF(TCSAFLUSH).c_lineABI byte across termios/termio round trips without panicking on unsupported values.tcdrain, close-time drain, output flush, blocking writes, poll, and epoll observe the actual software queue and transmitter state.Implementation details
Termios and legacy termio
PosixTermiorepresentation withNCC = 8.TCGETA.CIBAUDinput speeds, including the extended baud-rate values.N_TTY drain and flush semantics
TCSAFLUSHinput-only: pending output is drained, while unread input is discarded at the Linux-compatible point in the transaction.PTY behavior
TIOCOUTQoutput, while N_TTY output not yet accepted by the driver is still drained.serial8250 transmit and console path
write_room()andchars_in_buffer()values.tcdrainand close.NotHandledsemantics.Hangup and fasync lifecycle
hung_up_tty_fopsbehavior independently of later reopens.F_SETFLflag merging with fasync lifecycle updates so concurrent status-flag changes cannot restore staleFASYNCstate.ENOTTYfor unsupported ioctl dispatch instead of leaking the internalENOIOCTLCMDsentinel to userspace.Linux compatibility
The implementation was reviewed against Linux 6.6 behavior, including:
drivers/tty/tty_ioctl.cfor termios/termio conversion, job control, drain ordering, and input flush semantics;drivers/tty/n_tty.cfor retained output and write-wakeup behavior;drivers/tty/pty.cfor PTY write-room, drain, unthrottle, and hangup boundaries;drivers/tty/serial/serial_core.cand the 8250 driver for queued TX,wait_until_sent, flush wakeups, shutdown, and legacy IRQ routing;Validation
Completed locally:
make fmtand kernel clippy checks;make kernel;tty_termios: 14/14 passed;tty_tcflush: 6/6 passed;tty_pty_hangup: 35/35 passed;fcntl_signal: 7/7 passed;test_tty_termios.cstatic build;/dev/ttyS1transmitted 4096 bytes and the host received the complete payload without relying on COM1/IRQ4 traffic.The latest full Dunitest run reached an unrelated intermittent FAT page-cache writeback panic in code not changed by this PR. The FAT concurrency issue and CI evidence are tracked separately in #2158.
Change scope