feat(tty): Implement missing TTY termios ioctls (TCSETSF, TCSETA*)#2141
feat(tty): Implement missing TTY termios ioctls (TCSETSF, TCSETA*)#2141kaleidoscope416 wants to merge 22 commits into
Conversation
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.
|
@codex review |
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".
| core::mem::size_of::<PosixTermio>(), | ||
| true, | ||
| )?; | ||
| user_writer.copy_one_to_user(&termio, 0)?; |
There was a problem hiding this comment.
PosixTermio 的字段共 17 字节，但 repr(C) 按 u16 对齐后大小为 18 字节；这里的 copy_one_to_user 会通过 value_as_bytes 复制整个对象表示，包括未初始化的尾部填充。因此，任意可访问 TTY 的进程调用 TCGETA 时都可能获得一个内核栈残留字节。请显式添加并清零填充字段，或从已清零的 ABI 字节缓冲区逐字段序列化。
Useful? React with 👍 / 👎.
| // whether opost_pending was fully drained; a `false` means the | ||
| // output lock was held by a concurrent writer and the opost | ||
| // state could not be inspected (see drain_output docs). | ||
| let _ldisc_drained = ld.drain_output(tty.clone())?; |
There was a problem hiding this comment.
当并发写线程持有 output_lock，或驱动暂时无空间而使 drain_opost_pending 返回 false 时，这里直接丢弃结果并继续设置 termios；对于 PTY 或默认 chars_in_buffer() 返回 0 的驱动，后续检查会立即结束，使 TCSETSW/TCSETSF/TCSETAW/TCSETAF 在旧输出仍滞留于行规程时成功返回。应可中断地等待输出可继续并重试，直到 drain_output() 明确返回 true。
Useful? React with 👍 / 👎.
| const DRAIN_RETRIES: u32 = 100; | ||
| for _ in 0..DRAIN_RETRIES { | ||
| if tty.chars_in_buffer(tty.core()) == 0 { | ||
| break; | ||
| } | ||
| crate::sched::sched_yield(); |
There was a problem hiding this comment.
在低波特率串口、较长发送队列或流控暂停的设备上，硬件缓冲区很容易在 100 次 sched_yield() 后仍非空，但循环耗尽后仍会应用新 termios 并返回成功，违反带 TERMIOS_WAIT ioctl 必须等待既有输出发送完成的语义，并可能让在途字节受到新波特率等参数影响。这里需要基于发送完成事件进行可中断等待，而不是用固定调度次数作为排空上限。
Useful? React with 👍 / 👎.
| TtyIoctlCmd::TCSETA => { | ||
| return TtyCore::core_set_termios( | ||
| real_tty, | ||
| VirtAddr::new(arg), | ||
| TtySetTermiosOpt::TERMIOS_TERMIO, |
There was a problem hiding this comment.
当后台进程组对其控制终端调用新增的 TCSETA/TCSETAW/TCSETAF 时，这些分支直接进入 core_set_termios，而 tty_device.rs 的预检查列表及该公共函数均未调用 tty_check_change(SIGTTOU)，所以后台任务可以直接修改终端属性而不会收到 Linux 作业控制要求的信号或错误。请在所有 termios 设置命令的公共入口统一执行该检查。
Useful? React with 👍 / 👎.
| // Unknown / unsupported line disciplines fall back to NTty, | ||
| // matching Linux behaviour (N_TTY is the default). | ||
| _ => Self::NTty, |
There was a problem hiding this comment.
当用户通过 TCSETS 或新实现的 TCSETA* 传入非零 c_line 时，这里会静默把它改成 NTty，导致随后的 TCGETS/TCGETA 返回 0 而非原值。Linux 将 termios 中的 c_line 作为原始字节往返保存，实际行规程切换由 TIOCSETD 负责；请将 ABI 字段与内部活动行规程类型分开存储。
Useful? React with 👍 / 👎.
…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.
|
@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".
| if !ldisc_drained { | ||
| log::debug!( | ||
| "TERMIOS_WAIT: output_lock held for {} retries, proceeding without full ldisc drain", | ||
| DRAIN_RETRIES | ||
| ); |
There was a problem hiding this comment.
当 TCSETSW/TCSETAW/TCSETSF/TCSETAF 与正在写 tty 的线程竞争 output_lock 时，drain_output() 会返回 false；这里仅记录日志并继续，而随后 chars_in_buffer() 只看驱动 FIFO（PTY 还会立即为 0），不覆盖 N_TTY 的 opost/echo pending。这样 tcsetattr(TCSADRAIN/TCSAFLUSH) 可在旧输出未排空时成功并切换 termios，破坏 drain 语义。
Useful? React with 👍 / 👎.
| let _ = write_wq.wait_event_interruptible( | ||
| (EPollEventType::EPOLLOUT.bits() | EPollEventType::EPOLLWRNORM.bits()) as u64, | ||
| || tty.chars_in_buffer(core_ref) == 0, | ||
| ); |
There was a problem hiding this comment.
当等待期间收到信号时，EventWaitQueue::wait_event_interruptible 会返回 ERESTARTSYS，但这里用 let _ = 丢弃错误并继续调用 set_termios_next；Linux 的 set_termios 在这种可中断等待失败时返回错误且不应用新 termios。否则可取消的 TCSADRAIN/TCSAFLUSH 调用会表现为成功并产生状态变更。
Useful? React with 👍 / 👎.
| let drained = self.drain_opost_pending(&tty)?; | ||
| self.drain_echoes(&tty)?; | ||
| Ok(drained) |
There was a problem hiding this comment.
当只剩 echo 输出，或底层 write 返回 0 使 drain_echoes() 在仍有 echo pending 时提前 Ok(())，这里仍按 drain_opost_pending() 的结果返回 true；上层因此会停止等待并切换 termios。需要让 echo drain 也返回是否真正排空，或在返回 true 前检查 echo pending。
Useful? React with 👍 / 👎.
| @@ -385,7 +436,47 @@ impl TtyCore { | |||
| } | |||
|
|
|||
| if opt.contains(TtySetTermiosOpt::TERMIOS_WAIT) { | |||
There was a problem hiding this comment.
对于 TCSETSF/TCSETAF（TCSAFLUSH），此等待块是在前面的 TERMIOS_FLUSH 之后才执行；如果 drain 期间有 typeahead 输入到达，这些输入不会再被丢弃，却应在属性变更前被 flush。应先完成 wait/drain，再执行 flush_buffer，否则依赖 TCSAFLUSH 清空未读输入的程序仍能读到等待期间到达的数据。
Useful? React with 👍 / 👎.
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)
1425432 to
c3eb3f4
Compare
- B1: document TOCTOU limitation between final drain wait and set_termios_next - B2: replace silent flush_buffer error discard with log::warn! - B3: zero-initialize struct termios in C test to avoid garbage on tcgetattr failure
P2-1: Replace CHECK with CHECK_NOERR for value comparisons in C test P2-2: Add c_cflag/c_oflag low-16-bit cross-checks in both C and C++ tests P2-3: Document DO_WRITE_WAKEUP dependency between drain_opost_pending and drain_echoes P3-4: Fix CLinelRoundtrip → CLineRoundtrip typo in C++ test P3-5: Add TCGETA readback verification after TCSETA/TCSETAW/TCSETAF P3-6: Return empty PtyPair on tcgetattr failure in OpenRawPty P3-7: Document best-effort echo policy in write_wakeup doc comment P3-8: Document _pad intentional ignorance in to_kernel_termios
2 participants
摘要
补全 TTY termios 相关 ioctl 处理，使
tcsetattr()及旧式struct termio路径行为与 Linux 6.6 对齐。修复
dpkg --configure/apt install期间debconf的报错：根因
kernel/src/driver/tty/tty_core.rs中tty_mode_ioctl()仅处理了 TCGETS / TCSETS / TCSETSW / TCSETSF。存在三处缺口：struct termioioctl 未接入ENOIOCTLCMD→ENOTTYTERMIOS_WAIT未实现// TODO→ 空操作LineDisciplineType::from_line()对非零输入 panicc_line != 0的 termios/termio 路径todo!()→ 内核崩溃变更
7db07072PosixTermio结构体（SVR4struct termio, NCC=8），与内核Termios的双向转换采用 合并语义，对齐 Linux 6.6user_termio_to_kernel_termios()a486f3efTtyLineDisciplinetrait 新增drain_output()方法，NTty 实现ed4f9cd3TERMIOS_TERMIO分支替换core_set_termios中的todo!()67953d43tty_mode_ioctl中接入 TCGETA / TCSETA / TCSETAW / TCSETAF5bb43624TERMIOS_WAIT排空：drain_output()+ 带sched_yield()的有界chars_in_buffer()轮询1391f91ftest_tty_termios.c，覆盖 TCSAFLUSH / TCSADRAIN / 旧式 termio1a44519eLineDisciplineType::from_line()非零值 panic，降级为 NTtyce0ffaa4Linux 6.6.21 对齐
PosixTermio::to_kernel_termios()保留当前 termios 的高 16 位，与user_termio_to_kernel_termios()完全对齐（drivers/tty/tty_ioctl.c:373）TtySetTermiosOpt标志与tty_ioctl.c:838-845一致set_termios()仅调用flush_buffer（输入），不调用flush_outputspeeds_from_cflag()镜像 Linuxset_termios()中tty_termios_baud_rate()的调用文件
kernel/src/driver/tty/termios.rskernel/src/driver/tty/tty_core.rskernel/src/driver/tty/tty_ldisc/mod.rskernel/src/driver/tty/tty_ldisc/ntty.rsuser/apps/c_unitest/test_tty_termios.c验证
cargo check --target=x86_64-unknown-none通过test_tty_termios.c以x86_64-linux-musl-gcc -static编译通过