⬢ DragonFlyBSD Kernel Audit
DF-5456 / fix.diff
← back to finding ↓ download raw
--- a/sys/bus/u4b/serial/umoscom.c
+++ b/sys/bus/u4b/serial/umoscom.c
@@ -54,6 +54,16 @@
 #endif
 
 #define	UMOSCOM_BUFSIZE	       1024	/* bytes */
+
+/*
+ * Bound the synchronous control-request timeout used from the ucom cfg
+ * callbacks: these run on the per-adapter ucom taskqueue and block
+ * close(2) (usb_proc_mwait) and the USB hub event thread during detach
+ * drain (usb_proc_drain), so a 1000 ms timeout per request lets a slow
+ * or malicious MCS7703 pin them for many seconds -- the first-open chain
+ * alone is 14 sequential requests (~14.2 s).
+ */
+#define	UMOSCOM_CFG_TIMEOUT	250
 
 #define	UMOSCOM_CONFIG_INDEX	0
 #define	UMOSCOM_IFACE_INDEX	0
@@ -201,7 +211,7 @@
 static void	umoscom_cfg_param(struct ucom_softc *, struct termios *);
 static void	umoscom_cfg_get_status(struct ucom_softc *, uint8_t *,
 		    uint8_t *);
-static void	umoscom_cfg_write(struct umoscom_softc *, uint16_t, uint16_t);
+static int	umoscom_cfg_write(struct umoscom_softc *, uint16_t, uint16_t);
 static uint8_t	umoscom_cfg_read(struct umoscom_softc *, uint16_t);
 static void	umoscom_start_read(struct ucom_softc *);
 static void	umoscom_stop_read(struct ucom_softc *);
@@ -365,22 +375,27 @@
 	DPRINTF("\n");
 
 	/* Purge FIFOs or odd things happen */
-	umoscom_cfg_write(sc, UMOSCOM_FIFO, 0x00 | UMOSCOM_UART_REG);
+	if (umoscom_cfg_write(sc, UMOSCOM_FIFO, 0x00 | UMOSCOM_UART_REG))
+		return;
 
 	/* Enable FIFO */
-	umoscom_cfg_write(sc, UMOSCOM_FIFO, UMOSCOM_FIFO_EN |
+	if (umoscom_cfg_write(sc, UMOSCOM_FIFO, UMOSCOM_FIFO_EN |
 	    UMOSCOM_FIFO_RXCLR | UMOSCOM_FIFO_TXCLR |
 	    UMOSCOM_FIFO_DMA_BLK | UMOSCOM_FIFO_RXLVL_MASK |
-	    UMOSCOM_UART_REG);
+	    UMOSCOM_UART_REG))
+		return;
 
 	/* Enable Interrupt Registers */
-	umoscom_cfg_write(sc, UMOSCOM_INT, 0x0C | UMOSCOM_UART_REG);
+	if (umoscom_cfg_write(sc, UMOSCOM_INT, 0x0C | UMOSCOM_UART_REG))
+		return;
 
 	/* Magic */
-	umoscom_cfg_write(sc, 0x01, 0x08);
+	if (umoscom_cfg_write(sc, 0x01, 0x08))
+		return;
 
 	/* Magic */
-	umoscom_cfg_write(sc, 0x00, 0x02);
+	if (umoscom_cfg_write(sc, 0x00, 0x02))
+		return;
 }
 
 static void
@@ -399,7 +414,8 @@
 	if (onoff)
 		val |= UMOSCOM_LCR_BREAK;
 
-	umoscom_cfg_write(sc, UMOSCOM_LCR, val | UMOSCOM_UART_REG);
+	if (umoscom_cfg_write(sc, UMOSCOM_LCR, val | UMOSCOM_UART_REG))
+		DPRINTF("LCR break write failed\n");
 }
 
 static void
@@ -412,7 +428,9 @@
 	else
 		sc->sc_mcr &= ~UMOSCOM_MCR_DTR;
 
-	umoscom_cfg_write(sc, UMOSCOM_MCR, sc->sc_mcr | UMOSCOM_UART_REG);
+	if (umoscom_cfg_write(sc, UMOSCOM_MCR,
+	    sc->sc_mcr | UMOSCOM_UART_REG))
+		DPRINTF("MCR write failed\n");
 }
 
 static void
@@ -425,7 +443,9 @@
 	else
 		sc->sc_mcr &= ~UMOSCOM_MCR_RTS;
 
-	umoscom_cfg_write(sc, UMOSCOM_MCR, sc->sc_mcr | UMOSCOM_UART_REG);
+	if (umoscom_cfg_write(sc, UMOSCOM_MCR,
+	    sc->sc_mcr | UMOSCOM_UART_REG))
+		DPRINTF("MCR write failed\n");
 }
 
 static int
@@ -451,14 +471,24 @@
 		DPRINTF("invalid baud rate!\n");
 		return;
 	}
-	umoscom_cfg_write(sc, UMOSCOM_LCR,
-	    UMOSCOM_LCR_DIVLATCH_EN | UMOSCOM_UART_REG);
-
-	umoscom_cfg_write(sc, UMOSCOM_BAUDLO,
-	    (data & 0xFF) | UMOSCOM_UART_REG);
-
-	umoscom_cfg_write(sc, UMOSCOM_BAUDHI,
-	    ((data >> 8) & 0xFF) | UMOSCOM_UART_REG);
+	/*
+	 * Abort on the first failed request; whenever we do get past the
+	 * divisor writes the final LCR write below still executes, so the
+	 * divisor latch never stays open.
+	 */
+	if (umoscom_cfg_write(sc, UMOSCOM_LCR,
+	    UMOSCOM_LCR_DIVLATCH_EN | UMOSCOM_UART_REG)) {
+		DPRINTF("divisor latch enable failed, aborting param\n");
+		return;
+	}
+
+	if (umoscom_cfg_write(sc, UMOSCOM_BAUDLO,
+	    (data & 0xFF) | UMOSCOM_UART_REG))
+		DPRINTF("divisor lo write failed\n");
+
+	if (umoscom_cfg_write(sc, UMOSCOM_BAUDHI,
+	    ((data >> 8) & 0xFF) | UMOSCOM_UART_REG))
+		DPRINTF("divisor hi write failed\n");
 
 	if (t->c_cflag & CSTOPB)
 		data = UMOSCOM_LCR_STOP_BITS_2;
@@ -489,7 +519,8 @@
 	}
 
 	sc->sc_lcr = data;
-	umoscom_cfg_write(sc, UMOSCOM_LCR, data | UMOSCOM_UART_REG);
+	if (umoscom_cfg_write(sc, UMOSCOM_LCR, data | UMOSCOM_UART_REG))
+		DPRINTF("LCR format write failed\n");
 }
 
 static void
@@ -521,7 +552,7 @@
 		*p_msr |= SER_DSR;
 }
 
-static void
+static int
 umoscom_cfg_write(struct umoscom_softc *sc, uint16_t reg, uint16_t val)
 {
 	struct usb_device_request req;
@@ -532,8 +563,8 @@
 	USETW(req.wIndex, reg);
 	USETW(req.wLength, 0);
 
-	ucom_cfg_do_request(sc->sc_udev, &sc->sc_ucom,
-	    &req, NULL, 0, 1000);
+	return (ucom_cfg_do_request(sc->sc_udev, &sc->sc_ucom,
+	    &req, NULL, 0, UMOSCOM_CFG_TIMEOUT));
 }
 
 static uint8_t
@@ -549,7 +580,7 @@
 	USETW(req.wLength, 1);
 
 	ucom_cfg_do_request(sc->sc_udev, &sc->sc_ucom, 
-	    &req, &val, 0, 1000);
+	    &req, &val, 0, UMOSCOM_CFG_TIMEOUT);
 
 	DPRINTF("reg=0x%04x, val=0x%02x\n", reg, val);