lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 29 Mar 2017 20:44:31 +0200
From:   Olliver Schinagl <oliver@...inagl.nl>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        Stephen Warren <swarren@...dotorg.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Alexandre Courbot <gnurou@...il.com>,
        "David S . Miller" <davem@...emloft.net>
Cc:     dev@...ux-sunxi.org, Ed Blake <ed.blake@...tec.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Alexander Sverdlin <alexander.sverdlin@...ia.com>,
        Yegor Yefremov <yegorslists@...glemail.com>,
        Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@...el.com>,
        Kefeng Wang <wangkefeng.wang@...wei.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Heiko Stuebner <heiko@...ech.de>,
        Jason Uy <jason.uy@...adcom.com>,
        Douglas Anderson <dianders@...omium.org>,
        Peter Hurley <peter@...leysoftware.com>,
        Tony Lindgren <tony@...mide.com>, Vignesh R <vigneshr@...com>,
        Thor Thayer <tthayer@...nsource.altera.com>,
        David Lechner <david@...hnology.com>,
        Jan Kiszka <jan.kiszka@...mens.com>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-tegra@...r.kernel.org, sparclinux@...r.kernel.org,
        Olliver Schinagl <oliver@...inagl.nl>
Subject: [PATCH] serial: Do not treat the IIR register as a bitfield

It seems that at some point, someone made the assumption that the UART
Interrupt ID Register was a bitfield and started to check if certain
bits where set.

Actually however the register contains interrupt ID's where only the MSB
seems to be used singular and the rest share at least one bit. Thus
doing bitfield operations is wrong.

This patch cleans up the serial_reg include file by ordering it and
replacing the UART_IIR_ID 'mask' with a proper mask for the register.
The OMAP uart appears to have used the two commonly 'reserved' bits 4
and 5 and thus get an UART_IIR_EXT_MASK for these two bits.

This patch then goes over all UART_IIR_* users and changes the code from
bitfield checking, to ID checking instead.

Signed-off-by: Olliver Schinagl <oliver@...inagl.nl>
---

Note, that I do not have all this hardware and used the fact that UART_IIR_*
yields ID's rather then bitfields and thus mentioned as above, cannot be
bit-checked. Please carefully look at the changes, as they do make sense to me
a slip up is quickly made. Additionally, note the old code worked probably 100%
of the time, but was written wrong I think.

If I was wrong, I do apologize for the noise.

 drivers/tty/serial/8250/8250_core.c |  8 +++++---
 drivers/tty/serial/8250/8250_dw.c   |  5 +++--
 drivers/tty/serial/8250/8250_fsl.c  |  3 ++-
 drivers/tty/serial/8250/8250_omap.c |  4 ++--
 drivers/tty/serial/8250/8250_port.c | 11 ++++++-----
 drivers/tty/serial/omap-serial.c    | 12 ++++++------
 drivers/tty/serial/pxa.c            |  2 +-
 drivers/tty/serial/serial-tegra.c   |  3 ++-
 drivers/tty/serial/sunsu.c          |  2 +-
 drivers/tty/serial/vr41xx_siu.c     |  2 +-
 include/uapi/linux/serial_reg.h     |  8 ++++----
 11 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 76e03a7de9cc..11b4e3fb0c1d 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -288,6 +288,7 @@ static void serial8250_backup_timeout(unsigned long data)
 	}
 
 	iir = serial_in(up, UART_IIR);
+	iir &= UART_IIR_MASK;
 
 	/*
 	 * This should be a safe test for anyone who doesn't trust the
@@ -297,14 +298,15 @@ static void serial8250_backup_timeout(unsigned long data)
 	 */
 	lsr = serial_in(up, UART_LSR);
 	up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
-	if ((iir & UART_IIR_NO_INT) && (up->ier & UART_IER_THRI) &&
+	if ((iir == UART_IIR_NO_INT) &&
+	    (up->ier & UART_IER_THRI) &&
 	    (!uart_circ_empty(&up->port.state->xmit) || up->port.x_char) &&
 	    (lsr & UART_LSR_THRE)) {
-		iir &= ~(UART_IIR_ID | UART_IIR_NO_INT);
+		iir &= ~(UART_IIR_MASK);
 		iir |= UART_IIR_THRI;
 	}
 
-	if (!(iir & UART_IIR_NO_INT))
+	if (iir != UART_IIR_NO_INT)
 		serial8250_tx_chars(up);
 
 	if (up->port.irq)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 56aded887771..90cf0e6b4c5b 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -218,7 +218,8 @@ static int dw8250_handle_irq(struct uart_port *p)
 	 * This problem has only been observed so far when not in DMA mode
 	 * so we limit the workaround only to non-DMA mode.
 	 */
-	if (!up->dma && ((iir & 0x3f) == UART_IIR_RX_TIMEOUT)) {
+	iir &= UART_IIR_MASK;
+	if (!up->dma && (iir == UART_IIR_RX_TIMEOUT)) {
 		spin_lock_irqsave(&p->lock, flags);
 		status = p->serial_in(p, UART_LSR);
 
@@ -231,7 +232,7 @@ static int dw8250_handle_irq(struct uart_port *p)
 	if (serial8250_handle_irq(p, iir))
 		return 1;
 
-	if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
+	if (iir == UART_IIR_BUSY) {
 		/* Clear the USR */
 		(void)p->serial_in(p, d->usr_reg);
 
diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c
index 910bfee5a88b..b866343bb485 100644
--- a/drivers/tty/serial/8250/8250_fsl.c
+++ b/drivers/tty/serial/8250/8250_fsl.c
@@ -33,7 +33,8 @@ int fsl8250_handle_irq(struct uart_port *port)
 	spin_lock_irqsave(&up->port.lock, flags);
 
 	iir = port->serial_in(port, UART_IIR);
-	if (iir & UART_IIR_NO_INT) {
+	iir &= UART_IIR_MASK;
+	if (iir == UART_IIR_NO_INT) {
 		spin_unlock_irqrestore(&up->port.lock, flags);
 		return 0;
 	}
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index e7e64913a748..ab7c2327d410 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -996,7 +996,7 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 
 static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
 {
-	switch (iir & 0x3f) {
+	switch (iir & (UART_IIR_MASK | UART_IIR_EXT_MASK)) {
 	case UART_IIR_RLSI:
 	case UART_IIR_RX_TIMEOUT:
 	case UART_IIR_RDI:
@@ -1021,7 +1021,7 @@ static int omap_8250_dma_handle_irq(struct uart_port *port)
 	serial8250_rpm_get(up);
 
 	iir = serial_port_in(port, UART_IIR);
-	if (iir & UART_IIR_NO_INT) {
+	if ((iir & UART_IIR_MASK) == UART_IIR_NO_INT) {
 		serial8250_rpm_put(up);
 		return 0;
 	}
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6119516ef5fc..8ee3204d63ba 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1806,7 +1806,7 @@ EXPORT_SYMBOL_GPL(serial8250_modem_status);
 
 static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
 {
-	switch (iir & 0x3f) {
+	switch (iir & UART_IIR_MASK) {
 	case UART_IIR_RX_TIMEOUT:
 		serial8250_rx_dma_flush(up);
 		/* fall-through */
@@ -1825,7 +1825,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
 	unsigned long flags;
 	struct uart_8250_port *up = up_to_u8250p(port);
 
-	if (iir & UART_IIR_NO_INT)
+	if ((iir & UART_IIR_MASK) == UART_IIR_NO_INT)
 		return 0;
 
 	spin_lock_irqsave(&port->lock, flags);
@@ -1896,7 +1896,7 @@ static int serial8250_tx_threshold_handle_irq(struct uart_port *port)
 	unsigned int iir = serial_port_in(port, UART_IIR);
 
 	/* TX Threshold IRQ triggered so load up FIFO */
-	if ((iir & UART_IIR_ID) == UART_IIR_THRI) {
+	if ((iir & UART_IIR_MASK) == UART_IIR_THRI) {
 		struct uart_8250_port *up = up_to_u8250p(port);
 
 		spin_lock_irqsave(&port->lock, flags);
@@ -2262,7 +2262,8 @@ int serial8250_do_startup(struct uart_port *port)
 		 * don't trust the iir, setup a timer to kick the UART
 		 * on a regular basis.
 		 */
-		if ((!(iir1 & UART_IIR_NO_INT) && (iir & UART_IIR_NO_INT)) ||
+		if ((((iir1 & UART_IIR_MASK) != UART_IIR_NO_INT) &&
+		     ((iir & UART_IIR_MASK) == UART_IIR_NO_INT)) ||
 		    up->port.flags & UPF_BUG_THRE) {
 			up->bugs |= UART_BUG_THRE;
 		}
@@ -2313,7 +2314,7 @@ int serial8250_do_startup(struct uart_port *port)
 	iir = serial_port_in(port, UART_IIR);
 	serial_port_out(port, UART_IER, 0);
 
-	if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT) {
+	if (lsr & UART_LSR_TEMT && ((iir & UART_IIR_MASK) == UART_IIR_NO_INT)) {
 		if (!(up->bugs & UART_BUG_TXEN)) {
 			up->bugs |= UART_BUG_TXEN;
 			pr_debug("ttyS%d - enabling bad tx status workarounds\n",
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6c6f82ad8d5c..4e7269b8bf1b 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -569,7 +569,6 @@ static irqreturn_t serial_omap_irq(int irq, void *dev_id)
 {
 	struct uart_omap_port *up = dev_id;
 	unsigned int iir, lsr;
-	unsigned int type;
 	irqreturn_t ret = IRQ_NONE;
 	int max_count = 256;
 
@@ -578,16 +577,15 @@ static irqreturn_t serial_omap_irq(int irq, void *dev_id)
 
 	do {
 		iir = serial_in(up, UART_IIR);
-		if (iir & UART_IIR_NO_INT)
+		iir &= (UART_IIR_MASK | UART_IIR_EXT_MASK);
+		if (iir == UART_IIR_NO_INT)
 			break;
 
 		ret = IRQ_HANDLED;
 		lsr = serial_in(up, UART_LSR);
 
 		/* extract IRQ type from IIR register */
-		type = iir & 0x3e;
-
-		switch (type) {
+		switch (iir) {
 		case UART_IIR_MSI:
 			check_modem_status(up);
 			break;
@@ -607,10 +605,12 @@ static irqreturn_t serial_omap_irq(int irq, void *dev_id)
 			break;
 		case UART_IIR_XOFF:
 			/* FALLTHROUGH */
+		case UART_IIR_BUSY:
+			/* FALLTHROUGH */
 		default:
 			break;
 		}
-	} while (!(iir & UART_IIR_NO_INT) && max_count--);
+	} while ((iir != UART_IIR_NO_INT) && max_count--);
 
 	spin_unlock(&up->port.lock);
 
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 905631df1f8b..37cb17a11b34 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -253,7 +253,7 @@ static inline irqreturn_t serial_pxa_irq(int irq, void *dev_id)
 	unsigned int iir, lsr;
 
 	iir = serial_in(up, UART_IIR);
-	if (iir & UART_IIR_NO_INT)
+	if ((iir & UART_IIR_MASK) == UART_IIR_NO_INT)
 		return IRQ_NONE;
 	spin_lock(&up->port.lock);
 	lsr = serial_in(up, UART_LSR);
diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index d92a150c8733..4a084161d1d2 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -691,7 +691,8 @@ static irqreturn_t tegra_uart_isr(int irq, void *data)
 	spin_lock_irqsave(&u->lock, flags);
 	while (1) {
 		iir = tegra_uart_read(tup, UART_IIR);
-		if (iir & UART_IIR_NO_INT) {
+		iir &= UART_IIR_MASK;
+		if (iir == UART_IIR_NO_INT) {
 			if (is_rx_int) {
 				tegra_uart_handle_rx_dma(tup);
 				if (tup->rx_in_progress) {
diff --git a/drivers/tty/serial/sunsu.c b/drivers/tty/serial/sunsu.c
index 72df2e1b88af..74e195d4d17e 100644
--- a/drivers/tty/serial/sunsu.c
+++ b/drivers/tty/serial/sunsu.c
@@ -475,7 +475,7 @@ static irqreturn_t sunsu_serial_interrupt(int irq, void *dev_id)
 
 		spin_lock_irqsave(&up->port.lock, flags);
 
-	} while (!(serial_in(up, UART_IIR) & UART_IIR_NO_INT));
+	} while ((serial_in(up, UART_IIR) & UART_IIR_MASK) != UART_IIR_NO_INT);
 
 	spin_unlock_irqrestore(&up->port.lock, flags);
 
diff --git a/drivers/tty/serial/vr41xx_siu.c b/drivers/tty/serial/vr41xx_siu.c
index 439057e8107a..fc3b9bd920e9 100644
--- a/drivers/tty/serial/vr41xx_siu.c
+++ b/drivers/tty/serial/vr41xx_siu.c
@@ -429,7 +429,7 @@ static irqreturn_t siu_interrupt(int irq, void *dev_id)
 	port = (struct uart_port *)dev_id;
 
 	iir = siu_read(port, UART_IIR);
-	if (iir & UART_IIR_NO_INT)
+	if ((iir & UART_IIR_MASK) == UART_IIR_NO_INT)
 		return IRQ_NONE;
 
 	lsr = siu_read(port, UART_LSR);
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index 5db76880b4ad..489522389a10 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -31,18 +31,18 @@
 #define UART_IERX_SLEEP		0x10 /* Enable sleep mode */
 
 #define UART_IIR	2	/* In:  Interrupt ID Register */
-#define UART_IIR_NO_INT		0x01 /* No interrupts pending */
-#define UART_IIR_ID		0x0e /* Mask for the interrupt ID */
 #define UART_IIR_MSI		0x00 /* Modem status interrupt */
+#define UART_IIR_NO_INT		0x01 /* No interrupts pending */
 #define UART_IIR_THRI		0x02 /* Transmitter holding register empty */
 #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
 #define UART_IIR_RLSI		0x06 /* Receiver line status interrupt */
-
 #define UART_IIR_BUSY		0x07 /* DesignWare APB Busy Detect */
+#define UART_IIR_RX_TIMEOUT	0x0c /* DesignWare RX Timeout interrupt */
+#define UART_IIR_MASK		0x0f /* DesignWare IIR mask */
 
-#define UART_IIR_RX_TIMEOUT	0x0c /* OMAP RX Timeout interrupt */
 #define UART_IIR_XOFF		0x10 /* OMAP XOFF/Special Character */
 #define UART_IIR_CTS_RTS_DSR	0x20 /* OMAP CTS/RTS/DSR Change */
+#define UART_IIR_EXT_MASK	0x30 /* OMAP extended IIR mask */
 
 #define UART_FCR	2	/* Out: FIFO Control Register */
 #define UART_FCR_ENABLE_FIFO	0x01 /* Enable the FIFO */
-- 
2.11.0

Powered by blists - more mailing lists