NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
kern/60758: ttsetwater() sets t_hiwat above the output queue size, so ONLCR can emit a duplicate CR
>Number: 60758
>Category: kern
>Synopsis: ttsetwater() sets t_hiwat above the output queue size, so ONLCR can emit a duplicate CR
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: kern-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Mon Sep 21 00:20:00 +0000 2026
>Originator: Jay Howard
>Release: 11.0
>Organization:
>Environment:
NetBSD netbsd 11.0 NetBSD 11.0 (GENERIC) #0: Thu Jul 30 15:23:12 UTC 2026
mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/amd64/compile/GENERIC amd64
>Description:
WHAT HAPPENS
------------
Write a lot of plain text to a pty with ONLCR set (the default) and the reader
occasionally receives "\r\r\n" where it should receive "\r\n".
The text being written contains no carriage returns at all. Every CR on the
wire is one the tty inserted while translating a newline, and it should insert
exactly one per newline. Sometimes it inserts two. Whether it happens depends
on how big the write(2) calls are, not on the data: the same bytes written in
64-byte chunks come out correct, and in 16 KB chunks come out with extra CRs.
WHY
---
Two things combine, and it takes both.
1. The ONLCR translation is not atomic. ttyoutput() queues the CR with one
putc() and the LF with another. If the output queue fills between the two,
it returns the character, and ttwrite() reprocesses that same '\n' once there
is room -- re-entering the ONLCR branch and queueing a SECOND CR. (ttwrite()
jumps to its overflow label before the cp++/cc-- that would have consumed the
character.)
2. The flow control that should make (1) unreachable never fires, because
ttsetwater() can set t_hiwat ABOVE the output queue's actual capacity. The
writer is therefore never throttled and the queue really does reach 100%
full.
NO OTHER CURRENT BSD OR LINUX DOES THIS (tested, same reproducer)
-----------------------------------------------------------------
Linux 6.17.0-1022-azure one CR per newline at every write size
FreeBSD 15.1-RELEASE-p3 one CR per newline at every write size
OpenBSD 7.9 one CR per newline at every write size
NetBSD 11.0 DUPLICATES
macOS 27.0.0 (XNU, arm64) DUPLICATES
Full output from the three unaffected kernels is at the end of this report.
Each of them closes the window, in one of two ways:
* Linux -- reserves room for both bytes before emitting either, then writes the
pair in one call:
if (O_ONLCR(tty)) { if (space < 2) return -1; ... tty->ops->write(tty, "\r\n", 2); }
drivers/tty/n_tty.c, do_output_char(). Introduced by commit a88a69c91256
(2009), https://github.com/torvalds/linux/commit/a88a69c91256
* FreeBSD -- writes the pair as an all-or-nothing queue operation:
error = ttyoutq_write_nofrag(&tp->t_outq, "\r\n", 2);
sys/kern/tty_ttydisc.c. Arrived with the MPSAFE TTY layer, commit
bc093719ca47 (2008), https://github.com/freebsd/freebsd-src/commit/bc093719ca47
* OpenBSD -- keeps the queue from ever filling, by clamping the high water mark
against the queue's real capacity:
tp->t_hiwat = CLAMP(x, tp->t_outq.c_cn - TTHIWATMINSPACE, TTMINHIWAT);
#define TTHIWATMINSPACE 200 /* Min space above hiwat */
sys/kern/tty.c, ttsetwater(). Commit 47ac5982efe8 (2015),
https://github.com/openbsd/src/commit/47ac5982efe8
The OpenBSD case is the closest precedent: it still has the same non-atomic
ttyoutput() and the same ttwrite() retry -- its tty.c is literally a fork of
NetBSD's, RCS id and all -- and it does not reproduce, purely because after that
commit the queue can no longer reach capacity.
For accuracy: none of those three commits was aimed at duplicated CRs. Linux's
was fixing *lost* characters, FreeBSD's was a wholesale rewrite, and OpenBSD's
was about interrupt safety and pty deadlocks. The claim here is only what was
measured -- their current versions do not exhibit this -- plus the visible
reason why.
APPENDIX -- the three unaffected kernels, same reproducer
---------------------------------------------------------
OpenBSD 7.9
input: 1320000 bytes, 20000 LF, 0 CR
write size captured LF CR dup CRs verdict
64 1340000 20000 20000 0 ok
256 1340000 20000 20000 0 ok
1024 1340000 20000 20000 0 ok
4096 1340000 20000 20000 0 ok
16384 1340000 20000 20000 0 ok
65536 1340000 20000 20000 0 ok
64K, raw 1320000 20000 0 0 control: OPOST off
NOT AFFECTED: exactly one CR per newline at every write size
FreeBSD 15.1-RELEASE-p3
input: 1320000 bytes, 20000 LF, 0 CR
write size captured LF CR dup CRs verdict
64 1340000 20000 20000 0 ok
256 1340000 20000 20000 0 ok
1024 1340000 20000 20000 0 ok
4096 1340000 20000 20000 0 ok
16384 1340000 20000 20000 0 ok
65536 1340000 20000 20000 0 ok
64K, raw 1320000 20000 0 0 control: OPOST off
NOT AFFECTED: exactly one CR per newline at every write size
Linux 6.17.0-1022-azure
input: 1320000 bytes, 20000 LF, 0 CR
write size captured LF CR dup CRs verdict
64 1340000 20000 20000 0 ok
256 1340000 20000 20000 0 ok
1024 1340000 20000 20000 0 ok
4096 1340000 20000 20000 0 ok
16384 1340000 20000 20000 0 ok
65536 1340000 20000 20000 0 ok
64K, raw 1320000 20000 0 0 control: OPOST off
NOT AFFECTED: exactly one CR per newline at every write size
>How-To-Repeat:
Python 3, no arguments, no dependencies. Writes one CR-free stream to a pty at
three write sizes and counts "\r\r\n". LF must stay at 20000 and CR must equal
it. (A C version is available if preferred.)
import os, pty, select
D = b"x" * 60 + b"\n" # one 61-byte line, no CR anywhere
N = 20000
def run(chunk):
pid, fd = pty.fork()
if pid == 0: # child: write the stream to the tty
b = D * N
while b:
n = os.write(1, b[:chunk]); b = b[n:]
os._exit(0)
out = bytearray()
while select.select([fd], [], [], 5)[0]:
try: c = os.read(fd, 65536)
except OSError: break
if not c: break
out += c
os.waitpid(pid, 0)
return bytes(out)
for chunk in (64, 4096, 65536):
o = run(chunk)
print("write %-6d LF=%d CR=%d '\\r\\r\\n'=%d" % (chunk, o.count(b"\n"), o.count(b"\r"), o.count(b"\r\r\n")))
print("input had 0 CR and %d LF; each LF must yield exactly one CR" % N)
On NetBSD 11.0 (amd64 GENERIC, under QEMU), the C version of the same test:
NetBSD netbsd 11.0 NetBSD 11.0 (GENERIC) #0: Thu Jul 30 15:23:12 UTC 2026 mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/amd64/compile/GENERIC amd64
kern.tty.qsize = 1024
input: 1320000 bytes, 20000 LF, 0 CR
write size captured LF CR dup CRs verdict
64 1340000 20000 20000 0 ok
256 1340001 20000 20001 1 DUPLICATED
1024 1340001 20000 20001 1 DUPLICATED
4096 1340001 20000 20001 1 DUPLICATED
16384 1340001 20000 20001 1 DUPLICATED
65536 1340001 20000 20001 1 DUPLICATED
64K, raw 1320000 20000 0 0 control: OPOST off
AFFECTED: the tty duplicated CRs
THE NUMBERS ON NETBSD
---------------------
On a default pty the guard cannot fire at all -- this is not a narrow race.
With kern.tty.qsize = 1024 and TTYDEF_SPEED (B9600):
cps = 9600 / 10 = 960
t_lowat = CLAMP(480, 256, 32) = 256
x = CLAMP(256 + 960, 2048, 128) = 1216 (TTMAXHIWAT = t_qsize << 1)
t_hiwat = roundup(1216, 64) = 1216
t_hiwat is 1216 for a queue that holds 1024 bytes, so ttwrite()'s
"tp->t_outq.c_cc > hiwat" test can never be true. At B1200, t_hiwat works out
to 192, which is under the queue size, the guard does fire, and the duplication
stops -- which is what the reproducer shows when the speed is changed.
IMPACT
------
Low. A terminal renders "\r\r\n" and "\r\n" identically, and a log written by an
application never crosses the tty, so this is only visible to something
capturing the pty stream byte for byte -- script(1), expect, tmux capture, CI
logs -- or to a program diffing its own output. I found it the latter way,
chasing a 21-byte difference between two builds of a MUD client.
>Fix:
I have not built or tested a patched kernel, so the following is analysis rather
than a patch. Which direction is right is better judged by someone who knows
what the serial drivers expect.
The narrow change is to make the ONLCR pair all-or-nothing in ttyoutput(), so a
partial pair cannot be emitted regardless of how the water marks are set:
check that t_outq has room for two characters before queueing the CR -- using
tp->t_outq.c_cn - tp->t_outq.c_cc, the free-space expression already used at
tty.c:1112 -- and return c otherwise. That return is not a new state; it is
exactly what the existing putc('\r') failure returns from the same branch, so
ttwrite()'s retry path already handles it. This is the shape Linux and FreeBSD
both use.
The root-cause change is to stop ttsetwater() setting t_hiwat above the output
queue's capacity, which is what makes the above reachable at all. OpenBSD did
this in 47ac5982efe8 by clamping against tp->t_outq.c_cn - TTHIWATMINSPACE.
Porting that is a small edit to the same function, but it retunes write
throttling for every tty including real serial ports, which I cannot test from
here -- and OpenBSD's own commit message notes that mishandling hiwat caused
deadlocks in pty, so it warrants more care than an outside patch deserves.
Doing both would also cover any other path that can leave the queue one byte
from full.
Home |
Main Index |
Thread Index |
Old Index