The naive programmer, having some experience with the tty, and having read the POSIX 2017 may assume that upon input of the EOF cchar just throws it self away and flushes the dumb line editor's contents to the read queue. That's not what happens in FreeBSD. If a canonical input processing program, like cat(1), chooses to repeatedly read() with nbyte set to 5 bytes, when the user presses 'h' 'e' 'l' 'l' 'o' '^D' successively, the read queue will be 6 bytes as reported by FIONREAD. The first read() will complete as expected, returning 5 and decreasing read queue to 1 byte. but when it finishes echoing the chars and read() again, the ttydisc would throw away the 1 byte in the input queue and immediately return 0! The program would then terminate, much to the surprise of the naive programmer. That is, when program read() the input queue when the sole character there is an EOF, the kernel would incorrectly return 0 rather than throw it away and wait for input. The EOF or canonical mode should not affect the processing of the input queue, only the dumb line editor. Normally read() returning 0 in canonical input processing should only happen when user have just pressed EOF/EOL/EOL2 and pressed EOF again. If I stop the program via debugger, hit 'hello^D', switch to -icanon via stty -f, then continue the program, this weirdness will not happen. The latest Linux kernel does not have this problem. NetBSD kernel also has this problem.
a minimal example, try to press 'hello^D' and observe it terminating surprisingly. ``` #include <unistd.h> int main() { char buffer[5]; ssize_t nread; while ((nread = read(STDIN_FILENO, buffer, 5)) > 0) { write(STDOUT_FILENO, buffer, nread); } return 0; } ```
Created attachment 247547 [details] git(1) diff against base I agree with your interpretation of the spec; I think we want something like the attached patch... it searches one past the buffer size for an EOF. If we find it there, then we will trim it; if we don't, then the next read() will return the remaining portion of the line and optionally trim the EOF (if found). The tty_inq.c portion of the patch just fixes a bogus assertion; if you read later in the function, you'll note that `clen` (thus, `rlen`) is assumed to include `flen`, so we shouldn't bomb out if we're trimming the excess past the remainder of the buffer.
I am not very good at coding, but that looks great!
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=d51dac5f1370bdca1ea20c6b48cdea463f6f5dda commit d51dac5f1370bdca1ea20c6b48cdea463f6f5dda Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2024-01-16 02:55:58 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2024-01-16 02:55:58 +0000 kern: tty: fix EOF handling for canonical reads If the read(2) buffer is one byte short of an EOF, then we'll end up reading the line into the buffer, then re-entering and seeing an EOF at the beginning of the inq, assuming it's a zero-length line. Fix this corner-case by searching one more byte than we have available for an EOF. If we found it, then we'll trim it here; otherwise, we'll limit our read to just the space we have in the out buffer and the next read(2) will (potentially) read the remainder of the line. Fix FIONREAD while we're here to match what an application can expect read(2) to return -- scan for the first break character in the part of the input that's been canonicalized, we'll never return more than that. PR: 276220 Reviewed by: cy, imp (both previous version), kib Differential Revision: https://reviews.freebsd.org/D43378 sys/kern/tty.c | 2 +- sys/kern/tty_ttydisc.c | 72 ++++++++++++++++++++++++++++++++++++++++++++------ sys/sys/ttydisc.h | 1 + 3 files changed, 66 insertions(+), 9 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=2a05c20339082c19e64f9027f80706b8d24a082d commit 2a05c20339082c19e64f9027f80706b8d24a082d Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2024-01-16 02:55:58 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2024-01-30 17:11:09 +0000 kern: tty: fix EOF handling for canonical reads If the read(2) buffer is one byte short of an EOF, then we'll end up reading the line into the buffer, then re-entering and seeing an EOF at the beginning of the inq, assuming it's a zero-length line. Fix this corner-case by searching one more byte than we have available for an EOF. If we found it, then we'll trim it here; otherwise, we'll limit our read to just the space we have in the out buffer and the next read(2) will (potentially) read the remainder of the line. Fix FIONREAD while we're here to match what an application can expect read(2) to return -- scan for the first break character in the part of the input that's been canonicalized, we'll never return more than that. PR: 276220 Reviewed by: cy, imp (both previous version), kib (cherry picked from commit d51dac5f1370bdca1ea20c6b48cdea463f6f5dda) sys/kern/tty.c | 2 +- sys/kern/tty_ttydisc.c | 72 ++++++++++++++++++++++++++++++++++++++++++++------ sys/sys/ttydisc.h | 1 + 3 files changed, 66 insertions(+), 9 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=8fb7d0ddd3e3b4af91f10536b6f307f8f8792190 commit 8fb7d0ddd3e3b4af91f10536b6f307f8f8792190 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2024-01-16 02:55:58 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2024-01-30 17:11:24 +0000 kern: tty: fix EOF handling for canonical reads If the read(2) buffer is one byte short of an EOF, then we'll end up reading the line into the buffer, then re-entering and seeing an EOF at the beginning of the inq, assuming it's a zero-length line. Fix this corner-case by searching one more byte than we have available for an EOF. If we found it, then we'll trim it here; otherwise, we'll limit our read to just the space we have in the out buffer and the next read(2) will (potentially) read the remainder of the line. Fix FIONREAD while we're here to match what an application can expect read(2) to return -- scan for the first break character in the part of the input that's been canonicalized, we'll never return more than that. PR: 276220 Reviewed by: cy, imp (both previous version), kib (cherry picked from commit d51dac5f1370bdca1ea20c6b48cdea463f6f5dda) sys/kern/tty.c | 2 +- sys/kern/tty_ttydisc.c | 72 ++++++++++++++++++++++++++++++++++++++++++++------ sys/sys/ttydisc.h | 1 + 3 files changed, 66 insertions(+), 9 deletions(-)
Merged to all stable branches, thanks!