Don't leave a fresh tty's c_cflag at zero by emkey1 · Pull Request #2783 · ish-app/ish

emkey1 · 2026-08-03T16:22:07Z

A fresh tty's c_cflag is 0, which reads back as B0 ("hang up"). ssh forwards the speed, and OpenBSD hangs up the login shell when it sees B0, so the session drops right after login (exit status 129). Reported on the iSH Discord by a user who couldn't log in to their OpenBSD server.

New ttys start at B38400|CS8|CREAD, like Linux's pty driver.
The CLI console translates the real terminal's c_cflag. Speeds Linux can't encode fall back to 38400.

Tested with tcgetattr on a pty, /dev/tty2, and the CLI console at 9600, 19200 cs7 parenb, 115200, and 7200.

d532698 gave a freshly allocated tty a sane c_cflag, but missed the second place one is built. termios_from_real() zero-initializes and then translates only the i/o/l flags and the control characters, never cflags -- so the CLI's console tty was re-zeroed immediately after tty_alloc had got it right, and still reported B0 with CS5 and CREAD clear. The app never showed this because its terminal is a pty, which tty_alloc covers on its own; only the CLI goes through tty-real.c. It matters anyway, since ssh(1) run from the CLI forwards the same ospeed 0 that hung up the BSD login shell in d532698. The host's c_cflag is not worth translating: iSH models no baud rate or character size, and BSD keeps the speed in a separate c_ospeed field rather than in CBAUD. Seed the same nominal default tty_alloc uses. busybox stty speed, CLI console tty: before 0 after 38400 fresh pty c_cflag (unchanged): 0677 = B38400|CS8|CREAD|HUPCL tests/manual/pty_line_discipline.c passes, default_cflag included. Found while porting d532698 upstream (ish-app#2783), where the same gap would have made the fix look ineffective to anyone testing on the command-line build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

sylvansab · 2026-09-15T20:08:51Z

Ping @tbodt Would this be merged ? It's the only thing I'd need to be able to log into my SSH box :-) Cheers

saagarjha · 2026-09-24T10:48:51Z

+#define CSIZE_ 0000060
+#define CS8_ 0000060
+#define CREAD_ 0000200
+// Note the kernel's termbits header spells these in hex: HUPCL is 0x400,


then why are these in octal

No good reason. Switched to hex, matching the kernel header.

saagarjha · 2026-09-24T10:49:23Z

+    // through a tty_driver of its own, but pty_open_fake registers them under
+    // TTY_PSEUDO_SLAVE_MAJOR, so to the guest they are pty slaves.
+    tty->termios.cflags = B38400_ | CS8_ | CREAD_;
+    if (type != TTY_PSEUDO_MASTER_MAJOR && type != TTY_PSEUDO_SLAVE_MAJOR)


Since these are fake flags I'm a little confused why the difference here

Fair, dropped it. Every tty now starts at B38400|CS8|CREAD.

saagarjha · 2026-09-24T10:49:51Z


 static struct termios_ termios_from_real(struct termios real) {
-    struct termios_ fake = {};
+    // The host's c_cflag is not translated: iSH models no baud rate or


I think we want to translate here, since it's literally hooking up the real terminal up

Done, it now translates the real terminal's c_cflag. Speeds Linux has no code for (like macOS's 7200) fall back to 38400.

…eport the real speed Brings in the version of ish-app#2783 revised after review: - fs/tty.h: the c_cflag constants in hex, as the kernel header spells them, plus the rest of the POSIX bits and tty_baud_index(). - fs/tty-real.c: the CLI console copies the host terminal's c_cflag (CSIZE, CSTOPB, CREAD, PARENB, PARODD, HUPCL, CLOCAL and the speed) instead of seeding a fixed default. Speeds Linux has no code for, and B0, come in as 38400. - fs/tty.c: TCGETS2 reports the rate the CBAUD bits encode rather than a fixed 38400. A guest that set 9600 read 9600 back from TCGETS and 38400 from TCGETS2. - kernel/native_libc.c: native tcgetattr maps CSIZE, the other c_cflag bits and the speed instead of assuming CS8 and 38400. The console/pty HUPCL split in tty_alloc stays. It matches Linux, and upstream dropped it only to keep the patch simple. Tested: pty_line_discipline gains tcgets2_speed, which fails on the old binary (9600 and 115200 read back as 38400) and passes on alpine-arm64, devuan-amd64 and alpine-i386. With a host pty at 9600, 19200 cs7 parenb, 115200 -hupcl and 7200, the CLI console reads back c_cflag 0x4bd, 0x5ae, 0x10b2 and 0x4bf (7200 falls back to 38400), and native stty reports B9600, B19200 and B115200 where it used to say B38400. The xcodebuild iphoneos build succeeds with tty_speed_{from,to}_host in iSH-AOK.debug.dylib. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

saagarjha · 2026-09-25T09:59:35Z

+    FLAG(c, CLOCAL);
 #undef FLAG
+    switch (real.c_cflag & CSIZE) {
+        case CS5: fake.cflags |= CS5_; break;


Maybe I'm missing something, but why are these not using the macro?

FLAG tests a single bit, and CSIZE is a field: CS5 is 0 and CS8 is the whole mask. It does work bit by bit, though, because CS8 is CS6 | CS7 on both Linux and Darwin, so it's two FLAGs now.

No please do not rely on the size of CSIZE that is bad. I do not get why you can't use FLAG this explanation does not make sense. Presumably there is something wrong with it but the shape of this looks to me exactly like what FLAG does

Sorry, I explained that badly. FLAG sets x_ whenever any bit of x is set, and CS8 is the whole CSIZE mask, so FLAG(c, CS8) also fires for a 6- or 7-bit terminal: CS7 would come out as CS7|CS8, which is CS8. The switch compares the whole field, so I've gone back to it and dropped the bit trick.

Uh, I think it was designed to support single-bit flags, of which CS8 is not? We should probably just make it more generic

saagarjha · 2026-09-25T10:02:53Z

    return NULL;
 }

+// Indexed by Linux baud code, like the kernel's baud_table. Codes above


I disagree with the kernel here, we should store this as two tables, it would make the code below much clearer

Done, split into two tables.

Now that I look at this again I think the better way to write it is the one loop we had before (with one array, which should hopefully make it more obvious you are duplicating values) but instead of checking against 15 you'd check against the actual value that gets us into the ex range

Makes sense. Back to one array and one loop, and it now checks against B38400_ instead of 15.

saagarjha · 2026-09-26T11:41:53Z

+    B2400, B4800, B9600, B19200, B38400,
+};
+static const speed_t real_speeds_ex[] = {
+    0, B57600, B115200, B230400,


Isn't 0 just B0?

Yes. It was only there to keep the indices lined up, and it's gone now that it's one array again.

saagarjha · 2026-09-26T11:46:15Z

    return NULL;
 }

+// Indexed by Linux baud code, like the kernel's baud_table. Codes above


Now that I look at this again I think the better way to write it is the one loop we had before (with one array, which should hopefully make it more obvious you are duplicating values) but instead of checking against 15 you'd check against the actual value that gets us into the ex range

saagarjha · 2026-09-26T11:47:35Z

+    FLAG(c, CLOCAL);
 #undef FLAG
+    switch (real.c_cflag & CSIZE) {
+        case CS5: fake.cflags |= CS5_; break;


No please do not rely on the size of CSIZE that is bad. I do not get why you can't use FLAG this explanation does not make sense. Presumably there is something wrong with it but the shape of this looks to me exactly like what FLAG does

Zero reads back as B0, which means hang up. ssh forwards the speed, and a BSD server then hangs up the session as soon as the login shell sets up the tty. Default to B38400|CS8|CREAD like Linux's pty driver, and translate the real terminal's c_cflag for the CLI's console. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

saagarjha · 2026-09-26T13:30:49Z

+    FLAG(c, CLOCAL);
 #undef FLAG
+    switch (real.c_cflag & CSIZE) {
+        case CS5: fake.cflags |= CS5_; break;


Uh, I think it was designed to support single-bit flags, of which CS8 is not? We should probably just make it more generic

saagarjha · 2026-09-26T14:02:09Z


+// Indexed by Linux baud code. Codes above B38400 have CBAUDEX set: B57600 is
+// CBAUDEX | 1.
+static const speed_t real_speeds[] = {


I've just realized that you are (poorly) just reimplementing the Linux defines. We don't want to do that. Make proper defines for this, and then match them up using a switch statement+some macros or whatever other mechanism you think is compact and readable

emkey1 · 2026-09-26T14:12:53Z

Thanks for taking another look. This is now the fourth shape asked of the same few lines: one table, then two, then one again, and now defines with a switch. The size field went from a switch to FLAG and back, and is now headed for a more generic FLAG. Each round was done as asked. I'll leave the branch as it stands for anyone who needs to log in to a BSD host from iSH, and step back from this one.

sylvansab · 2026-09-29T21:16:52Z

I'm not sure, it seems this has been merged ? In any case now iSH won't log out right after having logged in and I'll get a remote prompt, however attempting to send any command (or even just hitting Enter right away) will return "Connection to [host] closed."

emkey1 force-pushed the tty_cflag_not_zero branch 2 times, most recently from e571ef9 to ce0a0de Compare August 3, 2026 16:56

saagarjha reviewed Sep 24, 2026

View reviewed changes

emkey1 force-pushed the tty_cflag_not_zero branch from ce0a0de to 258b4d0 Compare September 24, 2026 11:45

saagarjha requested changes Sep 25, 2026

View reviewed changes

emkey1 force-pushed the tty_cflag_not_zero branch from 258b4d0 to f1f7c59 Compare September 26, 2026 08:33

saagarjha reviewed Sep 26, 2026

View reviewed changes

emkey1 force-pushed the tty_cflag_not_zero branch from f1f7c59 to 9ce1d09 Compare September 26, 2026 11:55

saagarjha reviewed Sep 26, 2026

View reviewed changes

emkey1 closed this Sep 26, 2026

Conversation

emkey1 commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sylvansab commented Sep 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saagarjha Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emkey1 commented Sep 26, 2026

Uh oh!

sylvansab commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

emkey1 commented Aug 3, 2026 •

edited

Loading

saagarjha Sep 25, 2026 •

edited

Loading