[PATCH stable v2 0/2] termios: Alpha BOTHER/IBSHIFT, tty_baudrate fix

[PATCH stable v2 0/2] termios: Alpha BOTHER/IBSHIFT, tty_baudrate fix

[H. Peter Anvin]

From: "H. Peter Anvin (Intel)" <hpa@zytor.com>

It turns out that Alpha is the only architecture that never
implemented BOTHER and IBSHIFT, which is otherwise ages old. This is
one thing that has held up glibc support for this feature (all other
architectures have supported these for about a decade, at least before
the current 3.2 glibc cutoff.)

Furthermore, in the process of dealing with this, I discovered that
the current code in tty_baudrate.c can read past the end of the
baud_table[] on Alpha and PowerPC. The second patch in this series
fixes that, but it also cleans up the code substantially by
auto-generating the table and, since all architectures now have them,
removing all conditionals for BOTHER and IBSHIFT existing.

Tagging for stable because these are concrete and immediate
problems. I have a much bigger update in process, nearly done, which
will clean up a lot of duplicated code and make the uapi headers
usable for libc, but that is not critical on the same level.

Tested on x86, compile-tested on Alpha. SPARC, and PowerPC.

v2: the CBAUDEX masking-as-fallback code was wrong, but it could never
    be exercised on any architecture anyway (gcc would not even emit it)
    so just remove it. There are thus no object code changes from v1.

---
 arch/alpha/include/asm/termios.h       |   8 +-
 arch/alpha/include/uapi/asm/ioctls.h   |   5 +
 arch/alpha/include/uapi/asm/termbits.h |  17 +++
 drivers/tty/.gitignore                 |   1 +
 drivers/tty/Makefile                   |  16 +++
 drivers/tty/bmacros.c                  |   2 +
 drivers/tty/tty_baudrate.c             | 182 ++++++++++++---------------------
 7 files changed, 114 insertions(+), 117 deletions(-)

Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eugene Syromiatnikov <esyr@redhat.com>
Cc: <linux-alpha@vger.kernel.org>
Cc: <linux-serial@vger.kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: <stable@vger.kernel.org>

[PATCH stable v2 1/2] arch/alpha, termios: implement BOTHER, IBSHIFT and termios2

[H. Peter Anvin]

From: "H. Peter Anvin (Intel)" <hpa@zytor.com>

Alpha has had c_ispeed and c_ospeed, but still set speeds in c_cflags
using arbitrary flags. Because BOTHER is not defined, the general
Linux code doesn't allow setting arbitrary baud rates, and because
CBAUDEX == 0, we can have an array overrun of the baud_rate[] table in
drivers/tty/tty_baudrate.c if (c_cflags & CBAUD) == 037.

Resolve both problems by #defining BOTHER to 037 on Alpha.

However, userspace still needs to know if setting BOTHER is actually
safe given legacy kernels (does anyone actually care about that on
Alpha anymore?), so enable the TCGETS2/TCSETS*2 ioctls on Alpha, even
though they use the same structure. Define struct termios2 just for
compatibility; it is the exact same structure as struct termios. In a
future patchset, this will be cleaned up so the uapi headers are
usable from libc.

Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eugene Syromiatnikov <esyr@redhat.com>
Cc: <linux-alpha@vger.kernel.org>
Cc: <linux-serial@vger.kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: <stable@vger.kernel.org>
---
 arch/alpha/include/asm/termios.h       |  8 +++++++-
 arch/alpha/include/uapi/asm/ioctls.h   |  5 +++++
 arch/alpha/include/uapi/asm/termbits.h | 17 +++++++++++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/alpha/include/asm/termios.h b/arch/alpha/include/asm/termios.h
index 6a8c53dec57e..b7c77bb1bfd2 100644
--- a/arch/alpha/include/asm/termios.h
+++ b/arch/alpha/include/asm/termios.h
@@ -73,9 +73,15 @@
 })
 
 #define user_termios_to_kernel_termios(k, u) \
-	copy_from_user(k, u, sizeof(struct termios))
+	copy_from_user(k, u, sizeof(struct termios2))
 
 #define kernel_termios_to_user_termios(u, k) \
+	copy_to_user(u, k, sizeof(struct termios2))
+
+#define user_termios_to_kernel_termios_1(k, u) \
+	copy_from_user(k, u, sizeof(struct termios))
+
+#define kernel_termios_to_user_termios_1(u, k) \
 	copy_to_user(u, k, sizeof(struct termios))
 
 #endif	/* _ALPHA_TERMIOS_H */
diff --git a/arch/alpha/include/uapi/asm/ioctls.h b/arch/alpha/include/uapi/asm/ioctls.h
index 3729d92d3fa8..dc8c20ac7191 100644
--- a/arch/alpha/include/uapi/asm/ioctls.h
+++ b/arch/alpha/include/uapi/asm/ioctls.h
@@ -32,6 +32,11 @@
 #define TCXONC		_IO('t', 30)
 #define TCFLSH		_IO('t', 31)
 
+#define TCGETS2		_IOR('T', 42, struct termios2)
+#define TCSETS2		_IOW('T', 43, struct termios2)
+#define TCSETSW2	_IOW('T', 44, struct termios2)
+#define TCSETSF2	_IOW('T', 45, struct termios2)
+
 #define TIOCSWINSZ	_IOW('t', 103, struct winsize)
 #define TIOCGWINSZ	_IOR('t', 104, struct winsize)
 #define	TIOCSTART	_IO('t', 110)		/* start output, like ^Q */
diff --git a/arch/alpha/include/uapi/asm/termbits.h b/arch/alpha/include/uapi/asm/termbits.h
index de6c8360fbe3..4575ba34a0ea 100644
--- a/arch/alpha/include/uapi/asm/termbits.h
+++ b/arch/alpha/include/uapi/asm/termbits.h
@@ -26,6 +26,19 @@ struct termios {
 	speed_t c_ospeed;		/* output speed */
 };
 
+/* Alpha has identical termios and termios2 */
+
+struct termios2 {
+	tcflag_t c_iflag;		/* input mode flags */
+	tcflag_t c_oflag;		/* output mode flags */
+	tcflag_t c_cflag;		/* control mode flags */
+	tcflag_t c_lflag;		/* local mode flags */
+	cc_t c_cc[NCCS];		/* control characters */
+	cc_t c_line;			/* line discipline (== c_cc[19]) */
+	speed_t c_ispeed;		/* input speed */
+	speed_t c_ospeed;		/* output speed */
+};
+
 /* Alpha has matching termios and ktermios */
 
 struct ktermios {
@@ -152,6 +165,7 @@ struct ktermios {
 #define B3000000  00034
 #define B3500000  00035
 #define B4000000  00036
+#define BOTHER    00037
 
 #define CSIZE	00001400
 #define   CS5	00000000
@@ -169,6 +183,9 @@ struct ktermios {
 #define CMSPAR	  010000000000		/* mark or space (stick) parity */
 #define CRTSCTS	  020000000000		/* flow control */
 
+#define CIBAUD	07600000
+#define IBSHIFT	16
+
 /* c_lflag bits */
 #define ISIG	0x00000080
 #define ICANON	0x00000100
-- 
2.17.1

Re: [PATCH stable v2 1/2] arch/alpha, termios: implement BOTHER, IBSHIFT and termios2

[Johan Hovold]

On Sun, Oct 07, 2018 at 09:06:19PM -0700, H. Peter Anvin wrote:
> From: "H. Peter Anvin (Intel)" <hpa@zytor.com>
> 
> Alpha has had c_ispeed and c_ospeed, but still set speeds in c_cflags
> using arbitrary flags. Because BOTHER is not defined, the general
> Linux code doesn't allow setting arbitrary baud rates, and because
> CBAUDEX == 0, we can have an array overrun of the baud_rate[] table in
> drivers/tty/tty_baudrate.c if (c_cflags & CBAUD) == 037.
> 
> Resolve both problems by #defining BOTHER to 037 on Alpha.
> 
> However, userspace still needs to know if setting BOTHER is actually
> safe given legacy kernels (does anyone actually care about that on
> Alpha anymore?), so enable the TCGETS2/TCSETS*2 ioctls on Alpha, even
> though they use the same structure. Define struct termios2 just for
> compatibility; it is the exact same structure as struct termios. In a
> future patchset, this will be cleaned up so the uapi headers are
> usable from libc.

Is this really needed? By defining BOTHER (and IBSHIFT which you forgot
to mention here) you are enabling arbitrary rates also through TCSETS on
alpha, right?

Johan

Re: [PATCH stable v2 1/2] arch/alpha, termios: implement BOTHER, IBSHIFT and termios2

[H. Peter Anvin]

On 10/8/18 8:38 AM, Johan Hovold wrote:
> On Sun, Oct 07, 2018 at 09:06:19PM -0700, H. Peter Anvin wrote:
>> From: "H. Peter Anvin (Intel)" <hpa@zytor.com>
>>
>> Alpha has had c_ispeed and c_ospeed, but still set speeds in c_cflags
>> using arbitrary flags. Because BOTHER is not defined, the general
>> Linux code doesn't allow setting arbitrary baud rates, and because
>> CBAUDEX == 0, we can have an array overrun of the baud_rate[] table in
>> drivers/tty/tty_baudrate.c if (c_cflags & CBAUD) == 037.
>>
>> Resolve both problems by #defining BOTHER to 037 on Alpha.
>>
>> However, userspace still needs to know if setting BOTHER is actually
>> safe given legacy kernels (does anyone actually care about that on
>> Alpha anymore?), so enable the TCGETS2/TCSETS*2 ioctls on Alpha, even
>> though they use the same structure. Define struct termios2 just for
>> compatibility; it is the exact same structure as struct termios. In a
>> future patchset, this will be cleaned up so the uapi headers are
>> usable from libc.
> 
> Is this really needed? By defining BOTHER (and IBSHIFT which you forgot
> to mention here) you are enabling arbitrary rates also through TCSETS on
> alpha, right?
> 

Yes, it's needed, not because the old ioctls won't work on NEW kernels, but
because Alpha is so far behind the times, *and* the OLD kernels are severely
broken if we pass BOTHER to them, we need a new ioctl number so we can
guarantee that we won't do anything that user space doesn't intend; this is
actually made far worse because if I read the code correctly, the kernel will
still report back BOTHER and the speed field set on a legacy kernel in
response to TCGETS, but the values will be completely bogus.

This means that glibc will need a workaround for Alpha only, and the new ioctl
numbers handles support for it.  gcc should be able to fold the code together,
since it should be able to detect that multiple branches of execution are
otherwise identical.

To micro-optimize, we could define TERMIOS_OLD as (CBAUDEX ? 8 : 0) in a
future (non-stable) patch.

We don't need to worry about it on PowerPC because PowerPC implemented this so
long ago, before the current glibc support threshold.

	-hpa

[PATCH stable v2 2/2] termios, tty/tty_baudrate.c: simplify, auto-generate baud table

[H. Peter Anvin]

From: "H. Peter Anvin (Intel)" <hpa@zytor.com>

Now when all architectures define BOTHER and IBSHIFT, we can
unconditionally rely on these constants. Furthermore, the code can be
significantly simplified in a number of places.

Rather than having two tables and needing to be able to keep them in
sync at all times, have one auto-generated table. This also lets us
avoid the fact that architectures that have CBAUDEX == 0 have BOTHER
in a different location that those that don't.

The code for masking CBAUDEX as a fallback is never exercised on any
architecture, because for all architectures, either the baud rate
table is completely defined for all CBAUD values, or CBAUDEX == 0, so
we can just remove it.

Finally, this patch avoids overrunning the baud_table[] for
architectures with CBAUDEX == 0.

Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eugene Syromiatnikov <esyr@redhat.com>
Cc: <linux-alpha@vger.kernel.org>
Cc: <linux-serial@vger.kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: <stable@vger.kernel.org>
---
 drivers/tty/.gitignore     |   1 +
 drivers/tty/Makefile       |  16 ++++
 drivers/tty/bmacros.c      |   2 +
 drivers/tty/tty_baudrate.c | 182 ++++++++++++++-----------------------
 4 files changed, 85 insertions(+), 116 deletions(-)
 create mode 100644 drivers/tty/.gitignore
 create mode 100644 drivers/tty/bmacros.c

diff --git a/drivers/tty/.gitignore b/drivers/tty/.gitignore
new file mode 100644
index 000000000000..d436c18a912c
--- /dev/null
+++ b/drivers/tty/.gitignore
@@ -0,0 +1 @@
+bmacros.h
diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
index c72cafdf32b4..489b222ab1ce 100644
--- a/drivers/tty/Makefile
+++ b/drivers/tty/Makefile
@@ -35,3 +35,19 @@ obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
 obj-$(CONFIG_VCC)		+= vcc.o
 
 obj-y += ipwireless/
+
+#
+# Rules for generating the baud rate table
+#
+CFLAGS_bmacros.o += -Wp,-dM
+
+$(obj)/tty_baudrate.o: $(obj)/bmacros.h
+
+quiet_cmd_bmacros = BMACROS $@
+      cmd_bmacros =  ( \
+	sed -nr -e 's/^.define B([0-9]+) .*$$/BTBL(B\1,\1)/p' $< \
+		| sort -n -k1.7 > $@ ) || (rm -f $@ ; echo false)
+
+targets += bmacros.h
+$(obj)/bmacros.h: $(obj)/bmacros.i FORCE
+	$(call if_changed,bmacros)
diff --git a/drivers/tty/bmacros.c b/drivers/tty/bmacros.c
new file mode 100644
index 000000000000..0af865ff00de
--- /dev/null
+++ b/drivers/tty/bmacros.c
@@ -0,0 +1,2 @@
+/* This file is used to extract the B... macros from header files */
+#include <linux/termios.h>
diff --git a/drivers/tty/tty_baudrate.c b/drivers/tty/tty_baudrate.c
index 7576ceace571..ec6d3d93ffac 100644
--- a/drivers/tty/tty_baudrate.c
+++ b/drivers/tty/tty_baudrate.c
@@ -9,43 +9,48 @@
 #include <linux/tty.h>
 #include <linux/export.h>
 
-
 /*
- * Routine which returns the baud rate of the tty
- *
- * Note that the baud_table needs to be kept in sync with the
- * include/asm/termbits.h file.
+ * Routine which returns the baud rate of the tty.  The B... constants
+ * are extracted from the appropriate header files via
+ * bmacros.c -> bmacros.h.
  */
-static const speed_t baud_table[] = {
-	0, 50, 75, 110, 134, 150, 200, 300, 600, 1200, 1800, 2400, 4800,
-	9600, 19200, 38400, 57600, 115200, 230400, 460800,
-#ifdef __sparc__
-	76800, 153600, 307200, 614400, 921600
-#else
-	500000, 576000, 921600, 1000000, 1152000, 1500000, 2000000,
-	2500000, 3000000, 3500000, 4000000
+
+/* CBAUD mask with CBAUDEX removed */
+#define CBAUDNX (CBAUD & ~CBAUDEX)
+
+/* Lowest bit in CBAUDEX, if any.  CBAUDEX is assumed contiguous. */
+#define CBAUDEL (CBAUDEX & ~(CBAUDEX << 1))
+#if CBAUDEL & (CBAUDEL-1)
+# error "CBAUDEX is not contiguous"
 #endif
-};
 
-#ifndef __sparc__
-static const tcflag_t baud_bits[] = {
-	B0, B50, B75, B110, B134, B150, B200, B300, B600,
-	B1200, B1800, B2400, B4800, B9600, B19200, B38400,
-	B57600, B115200, B230400, B460800, B500000, B576000,
-	B921600, B1000000, B1152000, B1500000, B2000000, B2500000,
-	B3000000, B3500000, B4000000
-};
-#else
-static const tcflag_t baud_bits[] = {
-	B0, B50, B75, B110, B134, B150, B200, B300, B600,
-	B1200, B1800, B2400, B4800, B9600, B19200, B38400,
-	B57600, B115200, B230400, B460800, B76800, B153600,
-	B307200, B614400, B921600
+/* Convert from B... constant to table position */
+#define BPOS(x) (((x) & CBAUDNX) + \
+		 (CBAUDEX ? ((x) & CBAUDEX)/CBAUDEL*(CBAUDNX+1) : 0))
+
+/* Convert from table position to B... constant */
+#define BVAL(x) (((x) & CBAUDNX) + (((x)/(CBAUDNX+1)*CBAUDEX) & CBAUDEX))
+
+/* Entry into the baud_table */
+#define BTBL(b,v) [BPOS(b)] = (v),
+
+static const speed_t baud_table[] = {
+#include "bmacros.h"
 };
-#endif
 
 static int n_baud_table = ARRAY_SIZE(baud_table);
 
+/* Helper function; will be called with cbaud already masked */
+static speed_t _tty_termios_baud_rate(unsigned int cbaud, speed_t bother_speed)
+{
+	/* Magic token for arbitrary speed via c_ispeed/c_ospeed */
+	if (cbaud == BOTHER)
+		return bother_speed;
+
+	cbaud = BPOS(cbaud);
+	return (cbaud >= n_baud_table) ? 0 : baud_table[cbaud];
+}
+
 /**
  *	tty_termios_baud_rate
  *	@termios: termios structure
@@ -60,24 +65,8 @@ static int n_baud_table = ARRAY_SIZE(baud_table);
 
 speed_t tty_termios_baud_rate(struct ktermios *termios)
 {
-	unsigned int cbaud;
-
-	cbaud = termios->c_cflag & CBAUD;
-
-#ifdef BOTHER
-	/* Magic token for arbitrary speed via c_ispeed/c_ospeed */
-	if (cbaud == BOTHER)
-		return termios->c_ospeed;
-#endif
-	if (cbaud & CBAUDEX) {
-		cbaud &= ~CBAUDEX;
-
-		if (cbaud < 1 || cbaud + 15 > n_baud_table)
-			termios->c_cflag &= ~CBAUDEX;
-		else
-			cbaud += 15;
-	}
-	return baud_table[cbaud];
+	return _tty_termios_baud_rate(termios->c_cflag & CBAUD,
+				      termios->c_ospeed);
 }
 EXPORT_SYMBOL(tty_termios_baud_rate);
 
@@ -95,28 +84,15 @@ EXPORT_SYMBOL(tty_termios_baud_rate);
 
 speed_t tty_termios_input_baud_rate(struct ktermios *termios)
 {
-#ifdef IBSHIFT
-	unsigned int cbaud = (termios->c_cflag >> IBSHIFT) & CBAUD;
-
-	if (cbaud == B0)
-		return tty_termios_baud_rate(termios);
-#ifdef BOTHER
-	/* Magic token for arbitrary speed via c_ispeed*/
-	if (cbaud == BOTHER)
-		return termios->c_ispeed;
-#endif
-	if (cbaud & CBAUDEX) {
-		cbaud &= ~CBAUDEX;
+	unsigned int cbaud;
+	speed_t bother_speed = termios->c_ispeed;
 
-		if (cbaud < 1 || cbaud + 15 > n_baud_table)
-			termios->c_cflag &= ~(CBAUDEX << IBSHIFT);
-		else
-			cbaud += 15;
+	cbaud = (termios->c_cflag >> IBSHIFT) & CBAUD;
+	if (cbaud == B0) {
+		cbaud = termios->c_cflag & CBAUD;
+		bother_speed = termios->c_ospeed;
 	}
-	return baud_table[cbaud];
-#else	/* IBSHIFT */
-	return tty_termios_baud_rate(termios);
-#endif	/* IBSHIFT */
+	return _tty_termios_baud_rate(cbaud, bother_speed);
 }
 EXPORT_SYMBOL(tty_termios_input_baud_rate);
 
@@ -145,38 +121,27 @@ EXPORT_SYMBOL(tty_termios_input_baud_rate);
 void tty_termios_encode_baud_rate(struct ktermios *termios,
 				  speed_t ibaud, speed_t obaud)
 {
-	int i = 0;
-	int ifound = -1, ofound = -1;
+	int i;
+	unsigned int ifound, ofound;
 	int iclose = ibaud/50, oclose = obaud/50;
-	int ibinput = 0;
+	bool ibinput = false;
 
-	if (obaud == 0)			/* CD dropped 		  */
+	if (obaud == 0)			/* CD dropped		  */
 		ibaud = 0;		/* Clear ibaud to be sure */
 
 	termios->c_ispeed = ibaud;
 	termios->c_ospeed = obaud;
 
-#ifdef IBSHIFT
-	if ((termios->c_cflag >> IBSHIFT) & CBAUD)
-		ibinput = 1;	/* An input speed was specified */
-#endif
-#ifdef BOTHER
+	ibinput = ((termios->c_cflag >> IBSHIFT) & CBAUD) != B0;
+
 	/* If the user asked for a precise weird speed give a precise weird
 	   answer. If they asked for a Bfoo speed they may have problems
 	   digesting non-exact replies so fuzz a bit */
 
-	if ((termios->c_cflag & CBAUD) == BOTHER) {
+	if ((termios->c_cflag & CBAUD) == BOTHER)
 		oclose = 0;
-		if (!ibinput)
-			iclose = 0;
-	}
 	if (((termios->c_cflag >> IBSHIFT) & CBAUD) == BOTHER)
 		iclose = 0;
-#endif
-	termios->c_cflag &= ~CBAUD;
-#ifdef IBSHIFT
-	termios->c_cflag &= ~(CBAUD << IBSHIFT);
-#endif
 
 	/*
 	 *	Our goal is to find a close match to the standard baud rate
@@ -184,43 +149,28 @@ void tty_termios_encode_baud_rate(struct ktermios *termios,
 	 *	match then report back the speed as a POSIX Bxxxx value by
 	 *	preference
 	 */
-
-	do {
+	ofound = ifound = BOTHER;
+	for (i = 0; i < n_baud_table; i++) {
 		if (obaud - oclose <= baud_table[i] &&
 		    obaud + oclose >= baud_table[i]) {
-			termios->c_cflag |= baud_bits[i];
-			ofound = i;
+			ofound = BVAL(i);
+			break;
 		}
-		if (ibaud - iclose <= baud_table[i] &&
-		    ibaud + iclose >= baud_table[i]) {
-			/* For the case input == output don't set IBAUD bits
-			   if the user didn't do so */
-			if (ofound == i && !ibinput)
-				ifound  = i;
-#ifdef IBSHIFT
-			else {
-				ifound = i;
-				termios->c_cflag |= (baud_bits[i] << IBSHIFT);
+	}
+	if (!ibinput) {
+		ifound = 0;
+	} else {
+		for (i = 0; i < n_baud_table; i++) {
+			if (ibaud - iclose <= baud_table[i] &&
+			    ibaud + iclose >= baud_table[i]) {
+				ifound  = BVAL(i);
+				break;
 			}
-#endif
 		}
-	} while (++i < n_baud_table);
+	}
 
-	/*
-	 *	If we found no match then use BOTHER if provided or warn
-	 *	the user their platform maintainer needs to wake up if not.
-	 */
-#ifdef BOTHER
-	if (ofound == -1)
-		termios->c_cflag |= BOTHER;
-	/* Set exact input bits only if the input and output differ or the
-	   user already did */
-	if (ifound == -1 && (ibaud != obaud || ibinput))
-		termios->c_cflag |= (BOTHER << IBSHIFT);
-#else
-	if (ifound == -1 || ofound == -1)
-		pr_warn_once("tty: Unable to return correct speed data as your architecture needs updating.\n");
-#endif
+	termios->c_cflag &= ~(CBAUD | (CBAUD << IBSHIFT));
+	termios->c_cflag |= ofound | (ifound << IBSHIFT);
 }
 EXPORT_SYMBOL_GPL(tty_termios_encode_baud_rate);
 
-- 
2.17.1

Re: [PATCH stable v2 2/2] termios, tty/tty_baudrate.c: simplify, auto-generate baud table

[Johan Hovold]

On Sun, Oct 07, 2018 at 09:06:20PM -0700, H. Peter Anvin wrote:
> From: "H. Peter Anvin (Intel)" <hpa@zytor.com>
> 
> Now when all architectures define BOTHER and IBSHIFT, we can
> unconditionally rely on these constants. Furthermore, the code can be
> significantly simplified in a number of places.
> 
> Rather than having two tables and needing to be able to keep them in
> sync at all times, have one auto-generated table. This also lets us
> avoid the fact that architectures that have CBAUDEX == 0 have BOTHER
> in a different location that those that don't.
> 
> The code for masking CBAUDEX as a fallback is never exercised on any
> architecture, because for all architectures, either the baud rate
> table is completely defined for all CBAUD values, or CBAUDEX == 0, so
> we can just remove it.
> 
> Finally, this patch avoids overrunning the baud_table[] for
> architectures with CBAUDEX == 0.

So we need a minimal fix for this only as this patch in particular
should not be backported to stable.

I'm not sure when I'll have time to review this one thoroughly, so
perhaps others can chime in meanwhile.

Johan

Re: [PATCH stable v2 2/2] termios, tty/tty_baudrate.c: simplify, auto-generate baud table

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

On 10/8/18 8:46 AM, Johan Hovold wrote:
>
> So we need a minimal fix for this only as this patch in particular
> should not be backported to stable.
> 
> I'm not sure when I'll have time to review this one thoroughly, so
> perhaps others can chime in meanwhile.
> 
> Johan
> 

OK.  In the past Greg has generally liked to avoid fixes which will diverge
from upstream (because code in stable which is not in upstream can make
debugging difficult), but this is the minimal patch as requested; which to
apply is up to Greg.

As far as reviewing the cleanup patch, I strongly recommend:

a) Looking at the resulting file, not at the patch. Most of the code is simply
   merging the input and output rate functions into a common help function,
   and restructuring the code to that the utterly bizarre coding of a for loop
   using a do { ... } while() loop with the initial condition set at variable
   declaration(!!) far from the loop itself.
b) Examine bmacros.h after a build.
c) Build drivers/tty/tty_baudrate.s.  You can directly examine the baud_table
   and verify that it is, indeed, correct for whatever architecture you build.

	-hpa

[-- Attachment #2: 0002-termios-tty-tty_baudrate.c-fix-buffer-overrun.patch --]
[-- Type: text/x-patch, Size: 2051 bytes --]

>From c8195635f63508f4c75ef553b4703c4cc3c750e2 Mon Sep 17 00:00:00 2001
From: "H. Peter Anvin" <hpa@zytor.com>
Date: Mon, 8 Oct 2018 09:32:00 -0700
Subject: [PATCH 02/16] termios, tty/tty_baudrate.c: fix buffer overrun

On architectures with CBAUDEX == 0 (Alpha and PowerPC), the code in tty_baudrate.c does
not do any limit checking on the tty_baudrate[] array, and in fact a
buffer overrun is possible on both architectures. Add a limit check to
prevent that situation.

This will be followed by a much bigger cleanup/simplification patch.

Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Requested-by: Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eugene Syromiatnikov <esyr@redhat.com>
Cc: <linux-alpha@vger.kernel.org>
Cc: <linux-serial@vger.kernel.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: <stable@vger.kernel.org>
---
 drivers/tty/tty_baudrate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_baudrate.c b/drivers/tty/tty_baudrate.c
index 7576ceace571..f438eaa68246 100644
--- a/drivers/tty/tty_baudrate.c
+++ b/drivers/tty/tty_baudrate.c
@@ -77,7 +77,7 @@ speed_t tty_termios_baud_rate(struct ktermios *termios)
 		else
 			cbaud += 15;
 	}
-	return baud_table[cbaud];
+	return cbaud >= n_baud_table ? 0 : baud_table[cbaud];
 }
 EXPORT_SYMBOL(tty_termios_baud_rate);
 
@@ -113,7 +113,7 @@ speed_t tty_termios_input_baud_rate(struct ktermios *termios)
 		else
 			cbaud += 15;
 	}
-	return baud_table[cbaud];
+	return cbaud >= n_baud_table ? 0 : baud_table[cbaud];
 #else	/* IBSHIFT */
 	return tty_termios_baud_rate(termios);
 #endif	/* IBSHIFT */
-- 
2.17.1

Re: [PATCH stable v2 0/2] termios: Alpha BOTHER/IBSHIFT, tty_baudrate fix

[Johan Hovold]

On Sun, Oct 07, 2018 at 09:06:18PM -0700, H. Peter Anvin wrote:
> From: "H. Peter Anvin (Intel)" <hpa@zytor.com>
> 
> It turns out that Alpha is the only architecture that never
> implemented BOTHER and IBSHIFT, which is otherwise ages old. This is
> one thing that has held up glibc support for this feature (all other
> architectures have supported these for about a decade, at least before
> the current 3.2 glibc cutoff.)
> 
> Furthermore, in the process of dealing with this, I discovered that
> the current code in tty_baudrate.c can read past the end of the
> baud_table[] on Alpha and PowerPC. The second patch in this series
> fixes that, but it also cleans up the code substantially by
> auto-generating the table and, since all architectures now have them,
> removing all conditionals for BOTHER and IBSHIFT existing.
> 
> Tagging for stable because these are concrete and immediate
> problems.

This isn't stable material in its current form. If you want to plug the
alpha and powerpc info leaks in the stable trees, then you need a
minimal fix for that, which you can then your clean ups and new features
on.

Johan