Date: Wed, 8 Jul 2015 14:37:09 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: John-Mark Gurney <jmg@funkthat.com> Cc: Bruce Evans <brde@optusnet.com.au>, Ian Lepore <ian@freebsd.org>, Neel Natu <neel@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r285217 - head/usr.sbin/bhyve Message-ID: <20150708125051.D1092@besplex.bde.org> In-Reply-To: <20150707180627.GD8523@funkthat.com> References: <201507061933.t66JXTtJ050058@repo.freebsd.org> <1436213492.1334.64.camel@freebsd.org> <20150707122407.O1017@besplex.bde.org> <20150707180627.GD8523@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 7 Jul 2015, John-Mark Gurney wrote:
> Bruce Evans wrote this message on Tue, Jul 07, 2015 at 16:11 +1000:
>> - tty_init_console() doesn't set CLOCAL in the lock state device. So
>> without fixation, bugs like the one in reset(1) break serial consoles.
>
> Here's a patch to fix that:
> Index: tty.c
> ===================================================================
> --- tty.c (revision 284880)
> +++ tty.c (working copy)
> @@ -858,6 +858,8 @@ tty_init_console(struct tty *tp, speed_t s)
> {
> struct termios *ti = &tp->t_termios_init_in;
> struct termios *to = &tp->t_termios_init_out;
> + struct termios *lti = &tp->t_termios_lock_in;
> + struct termios *lto = &tp->t_termios_lock_out;
>
> if (s != 0) {
> ti->c_ispeed = ti->c_ospeed = s;
> @@ -866,6 +868,8 @@ tty_init_console(struct tty *tp, speed_t s)
>
> ti->c_cflag |= CLOCAL;
> to->c_cflag |= CLOCAL;
> + lti->c_cflag |= CLOCAL;
> + lto->c_cflag |= CLOCAL;
> }
>
> /*
>
> I've tested that this fixes the issue (on a machine that doesn't have
> the bhyve fix yet)...
I have used (some version) the enclosed more complete fix for about 5
years.
> To me this seems a very good way to start it, as it still allows the
> sysadmin to change the lock device on the console after boot if they
> so choose...
Of course I agree with this, since I designed it to work like that in
1994. (Actually, I copied most of the locking semantics from Linux,
and added initial state devices to give more control.) The commit
message for this seems to be have lost in the merge from FreeBSD-1.
Here it is from the FreeBSD-1 version (sio 1.46 in FreeBD-1 is part
of 1.51 in FreeBSD-2).
X RCS file: /home/ncvs/src1/sys/i386/isa/sio.c,v
X Working file: sio.c
X head: 1.56
X ----------------------------
X revision 1.46
X date: 1994/05/30 03:13:36; author: ache; state: Exp; lines: +416 -436
X From Ache:
X Better kernel printout for multiport cards
X
X From Bde:
X o Get decls of tk_nin, etc. from "dkstat.h" (a stupid place).
X o Mallocate sio structs as well as tty structs.
X o FAKE_DCD() is now handled by setting CLOCAL in the initial
X and lock states.
X o Removed ifdefs for COM_BIDIR. It is now standard. Callin
X devices now act identically (except for bug fixes) to the
X old standard devices if the callout devices are not used.
X o New magic minors for initial and lock devices. Better macros
X to handle magic minors. "ls -l /dev" should show something
X like this:
X crw------- 1 uucp wheel 28, 0 May 26 01:18 /dev/ttyd0
X crw------- 1 uucp wheel 28, 1 May 21 11:25 /dev/ttyd1
X crw-r--r-- 1 root wheel 28, 32 May 15 19:36 /dev/ttyid0
X crw-r--r-- 1 root wheel 28, 33 May 15 19:36 /dev/ttyid1
X crw-r--r-- 1 root wheel 28, 64 May 15 19:36 /dev/ttyld0
X crw-r--r-- 1 root wheel 28, 65 May 15 19:36 /dev/ttyld1
X crw------- 1 uucp wheel 28,128 May 26 14:48 /dev/cua00
X crw------- 1 uucp wheel 28,129 May 26 18:57 /dev/cua01
X crw-r--r-- 1 root wheel 28,160 May 15 18:58 /dev/cuai00
X crw-r--r-- 1 root wheel 28,161 May 15 19:35 /dev/cuai01
X crw-r--r-- 1 root wheel 28,192 May 15 18:56 /dev/cual00
X crw-r--r-- 1 root wheel 28,193 May 15 19:35 /dev/cual01
X The initial and lock devices are controlled using commands like
X this:
X stty </dev/ttyid0 57600 crtscts
X stty </dev/ttyld0 57600 clocal crtscts
X stty </dev/cuai00 57600 crtscts
X stty </dev/cual00 57600 crtscts
X Here the port speed is changed from the default of 9600
X to 57600 and locked (any nonzero speed works as a lock).
X clocal defaults to off and is locked off for the callin
X device to avoid certain security holes. crtscts is changed
X from the default of off to on and locked on (flag bits are
X locked by setting the same bits in the lock port). It is
X only necessary to set the initial state for buggy programs
X that don't set it for themself. It is dangerous to set
X lock bits if the device cannot support them (e.g., locking
X crtscts on for a mouse port will probably hang the X server).
X However, it may be necessary to set lock bits for buggy
X programs that clear nonstandard bits.
X o Removed FIFO_TRIGGER. The fifo trigger level is now set
X automatically. It is always set to 1 for slow mouses. It
X is set to 14 for the very first open but drops back to 8
X for first opens and to as low as 1 if overruns occur. I
X haven't been able to load my system enough to get it to
X drop below 8. Changes to the levels are logged. Please
X report if the level ever drops below 8.
X o The fifo disable flag still works to disable the fifo
X completely. Please don't use it.
X o Removed ifdefs for 1.1R interrupt handling.
X o Removed ifdefs for non-mallocation of tty structs.
X o Use timeout instead of sleep to hold down DTR for long
X enough. Fixed races. Now opens have to sleep while
X DTR is being held down.
X o Urgent input character now depends on the line discipline.
X Previously it only worked for SLIP. Now it might work
X for PPP to reduce latency by half a tick.
X o Attempted to fix initialization of interrupts for 4ports
X (flag bit 0x1 means 4port-compatible, not multiport). The
X master interrupt control register and ~OUT2 (badly aka
X MCR_IENABLE) have to be initialized before probing for
X interrupts.
X o Rewrote sioopen() to simplify it and avoid races. Too
X many changes to list. Essentially, an open either succeeds
X immediately or sleeps until it can succeed immediately.
X After every sleep, all the conditions required for success
X are checked again by restarting the open from near the top.
X o Callout ports are implemented using trapdoor carrier again.
X You can program their initial state to set CLOCAL if you
X want.
X o Use new sleep address macros. Better names for sleep addresses.
X o Removed bidir ioctls. There are enough important races to
X worry about.
X o Special treatment for ioctls to new initial and lock state
X devices. These devices only support ioctls, not read, write
X or select. Separate devices work better than would selector
X bits like CIGNORE because ordinary stty(1) works on them, and
X it is impossible to open a callin/callout device while the
X device going in the other direction is active.
X o The DTR wait time as seen by ioctls it is now always in
X hundredths of a second (independent of hz).
X o Removed TB_*() macros. There is no point to only using them
X in sio.
X o Use ttwwakeup() to handle write-wakeups consistently.
X ----------------------------
The following patch is for FreeBSD-9:
Y diff -c2 ./kern/tty.c~ ./kern/tty.c
Y *** ./kern/tty.c~ Fri Mar 2 08:50:08 2012
Y --- ./kern/tty.c Sun May 3 03:31:19 2015
Y ***************
Y *** 822,829 ****
Y struct termios *t = &tp->t_termios_init_in;
Y
Y t->c_cflag = TTYDEF_CFLAG;
Y ! t->c_iflag = TTYDEF_IFLAG;
Y ! t->c_lflag = TTYDEF_LFLAG;
Y ! t->c_oflag = TTYDEF_OFLAG;
Y t->c_ispeed = TTYDEF_SPEED;
Y t->c_ospeed = TTYDEF_SPEED;
Y --- 1047,1062 ----
Y struct termios *t = &tp->t_termios_init_in;
Y
Y + /*
Y + * Default to raw mode. The defaults in <sys/ttydefaults.h> are
Y + * really getty's defaults for login terminals, so they must not
Y + * all be used here. It is most important to have echo flags off
Y + * initially to prevent echo wars before the echo flags can be
Y + * turned off.
Y + */
Y t->c_cflag = TTYDEF_CFLAG;
Y ! t->c_iflag = IGNBRK;
Y ! t->c_lflag = 0;
Y ! t->c_oflag = 0;
Y !
Y t->c_ispeed = TTYDEF_SPEED;
Y t->c_ospeed = TTYDEF_SPEED;
This fixes the defaults for non-consoles. Using this requires unbreaking
console drivers that don't call tty init_console(). These drivers
currently get the console defaults less the speed and CLOCAL settings on
the initial state devices. I.e., they get the default speed of 9600
(good for 1980's hardware), the wrong initial state for CLOCAL (off),
and other wrong settings shared with drivers that do call
tty init_console(). After fixing tty_init_console() (but not calling it),
they are also missing the fixes for CLOCAL and HUPCL. After fixing the
above (which is always called), but still not calling tty init_console(),
the above gives a very broken initial state for c[ilo]flag on consoles.
I think nothing much except the single user shell would notice the wrong
initial state. Init is clueless about terminals. It wants to just open
the console device and have it work for shells.
The above unbroken defaults are partly to support similarly clueless
programs that want to just open a (non-console serial) device and have
it work in raw mode. E.g., cat >/dev/cuad0 should work if the line
speeds match and the other side understands raw mode. It is partly
to handle the echo war. If ECHO defaults to on, then it gives a race
after open to turn it off in programs that don't want it.
Y ***************
Y *** 833,849 ****
Y }
Y
Y void
Y ! tty_init_console(struct tty *tp, speed_t s)
Y {
Y ! struct termios *ti = &tp->t_termios_init_in;
Y ! struct termios *to = &tp->t_termios_init_out;
The patch also fixes style bugs like these initializations in declarations.
Y
Y ! if (s != 0) {
Y ! ti->c_ispeed = ti->c_ospeed = s;
Y ! to->c_ispeed = to->c_ospeed = s;
Y ! }
Y
Y ! ti->c_cflag |= CLOCAL;
Y ! to->c_cflag |= CLOCAL;
Y }
Y
Y --- 1066,1115 ----
Y }
Y
Y + /*
Y + * There is no getty for the single-user shell, so we must do its job and
Y + * change the default flags to ones suitable for logins. Also, if requested
Y + * to, then change the speed to whatever it actually is and lock this, and
Y + * unconditionally set CLOCAL and lock this, and unconditionally clear
Y + * HUPCL and lock this.
Y + *
Y + * Callers must tell us their speed and have us lock it iff they have a
Y + * speed that matters.
Y + *
Y + * These settings of CLOCAL and HUPCL are normally backwards for logins,
Y + * but CLOCAL is enforced so that the console cannot block endlessly when
Y + * a naive application like syslogd(8) attempts to write to it using a
Y + * simple open/write/close sequence, and ~HUPCL is enforced to avoid hanging
Y + * up on close in such a sequence (when the console is not held open by
Y + * another thread). Output is normally discarded instead of blocking.
Y + * Most ad hoc writes to the console use wall(8)'s ttymsg() and that is
Y + * remarkably deficent in terminal handling (it has none, but depends on
Y + * O_NONBLOCK). Not hanging up is a smaller feature. It matters mainly
Y + * when the other side of the connection is naive and is not using CLOCAL
Y + * so the connection gets broken by our hangup. However, if the other side
Y + * is sophisicated then it might prefer to get signaled on all transitions
Y + * of carrier. This can be supported by changing the initial state device
Y + * after booting. We should be more careful about raising and lowering
Y + * carrier before we are ready and finished, respectively, so that handshakes
Y + * based on carrier work.
Y + */
Attempt to prevent this being broken again by documenting the function.
Document some of the bugs in syslogd() etc.
Give hints about deficiencies in carrier handling. I recently reduced
some old races for closing devices. The correct timing for handling
carrier drop is unclear. Many devices or drivers have buffered i/o
and/or delays delivering notifications of carrier drop, so the tty
layer may see carrier drop before receiving all the data. Careful
senders will delay dropping carrier for long after sending their data
-- long enough to handle the timing and bugs in all supported receivers.
Most senders aren't careful, so receivers should probably delay acting
on carrier drop although POSIX might specify no delay.
syslogd() (ttymsg()) is an example of a non-careful sender. The
CLOCAL/HUPCL settings for reduce its problems with races on carrier
changes by ignoring carrier changes. But problems still occur. CRTSCTS
should probably be locked off like HUPCL, although this may cause data
to be discarded instead of waited for endlessly.
Y void
Y ! tty_init_console(struct tty *tp, speed_t speed)
Y {
Y ! struct termios *tinit, *tlock;
Y
Y ! tinit = &tp->t_termios_init_in;
Y ! tlock= &tp->t_termios_lock_in;
Y ! if (speed != 0) {
Y ! tinit->c_ispeed = tinit->c_ospeed = speed;
Y ! tlock->c_ispeed = tlock->c_ospeed = speed;
Y ! }
speed == 0 unfortunately still has to be special, since many console
drivers don't know their speed and/or don't tell us. They get the
default of 9600 and work iff that is their speed.
Y ! tinit->c_cflag = (TTYDEF_CFLAG | CLOCAL) & ~HUPCL;
Y ! tlock->c_cflag = CLOCAL | HUPCL;
Y ! tinit->c_iflag = TTYDEF_IFLAG;
Y ! tinit->c_lflag = TTYDEF_LFLAG;
Y ! tinit->c_oflag = TTYDEF_OFLAG;
Set all the flags explicitly, since all except c_cflag are 0 initially
and setting them all is clearer.
Y
Y ! tp->t_termios_init_out = *tinit;
Y ! tp->t_termios_lock_out = *tlock;
Copying the whole structs is easier. It is a design principle that
their default initial and lock states are as identical as possible.
In fact, they are identical. This should often be changed by userland
according to userland config files.
Note that initial and lock state devices are almost completlely broken
in -current. The synchronization between them is completely broken.
This reduces them to not much more than places to hold different
initial and lock settings. The hack that forces CLOCAL on for
callout devices does little more than setting CLOCAL in the callout
device's initial state.
Y }
Y
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150708125051.D1092>