⬢ DragonFlyBSD Kernel Audit
DF-5421 / fix.diff
← back to finding ↓ download raw
--- a/sys/bus/u4b/serial/uchcom.c
+++ b/sys/bus/u4b/serial/uchcom.c
@@ -209,12 +209,12 @@
 static void	uchcom_start_write(struct ucom_softc *);
 static void	uchcom_stop_read(struct ucom_softc *);
 static void	uchcom_stop_write(struct ucom_softc *);
-static void	uchcom_update_version(struct uchcom_softc *);
+static usb_error_t uchcom_update_version(struct uchcom_softc *);
 static void	uchcom_convert_status(struct uchcom_softc *, uint8_t);
-static void	uchcom_update_status(struct uchcom_softc *);
-static void	uchcom_set_dtr_rts(struct uchcom_softc *);
+static usb_error_t uchcom_update_status(struct uchcom_softc *);
+static usb_error_t uchcom_set_dtr_rts(struct uchcom_softc *);
 static int	uchcom_calc_divider_settings(struct uchcom_divider *, uint32_t);
-static void	uchcom_set_baudrate(struct uchcom_softc *, uint32_t);
+static usb_error_t uchcom_set_baudrate(struct uchcom_softc *, uint32_t);
 static void	uchcom_poll(struct ucom_softc *ucom);
 
 static device_probe_t uchcom_probe;
@@ -366,7 +366,7 @@
  * low level i/o
  */
 
-static void
+static usb_error_t
 uchcom_ctrl_write(struct uchcom_softc *sc, uint8_t reqno,
     uint16_t value, uint16_t index)
 {
@@ -378,11 +378,11 @@
 	USETW(req.wIndex, index);
 	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, 1000));
 }
 
-static void
+static usb_error_t
 uchcom_ctrl_read(struct uchcom_softc *sc, uint8_t reqno,
     uint16_t value, uint16_t index, void *buf, uint16_t buflen)
 {
@@ -394,31 +394,34 @@
 	USETW(req.wIndex, index);
 	USETW(req.wLength, buflen);
 
-	ucom_cfg_do_request(sc->sc_udev,
-	    &sc->sc_ucom, &req, buf, USB_SHORT_XFER_OK, 1000);
+	return (ucom_cfg_do_request(sc->sc_udev,
+	    &sc->sc_ucom, &req, buf, USB_SHORT_XFER_OK, 1000));
 }
 
-static void
+static usb_error_t
 uchcom_write_reg(struct uchcom_softc *sc,
     uint8_t reg1, uint8_t val1, uint8_t reg2, uint8_t val2)
 {
 	DPRINTF("0x%02X<-0x%02X, 0x%02X<-0x%02X\n",
 	    (unsigned)reg1, (unsigned)val1,
 	    (unsigned)reg2, (unsigned)val2);
-	uchcom_ctrl_write(
+	return (uchcom_ctrl_write(
 	    sc, UCHCOM_REQ_WRITE_REG,
-	    reg1 | ((uint16_t)reg2 << 8), val1 | ((uint16_t)val2 << 8));
+	    reg1 | ((uint16_t)reg2 << 8), val1 | ((uint16_t)val2 << 8)));
 }
 
-static void
+static usb_error_t
 uchcom_read_reg(struct uchcom_softc *sc,
     uint8_t reg1, uint8_t *rval1, uint8_t reg2, uint8_t *rval2)
 {
 	uint8_t buf[UCHCOM_INPUT_BUF_SIZE];
+	usb_error_t err;
 
-	uchcom_ctrl_read(
+	err = uchcom_ctrl_read(
 	    sc, UCHCOM_REQ_READ_REG,
 	    reg1 | ((uint16_t)reg2 << 8), 0, buf, sizeof(buf));
+	if (err)
+		return (err);
 
 	DPRINTF("0x%02X->0x%02X, 0x%02X->0x%02X\n",
 	    (unsigned)reg1, (unsigned)buf[0],
@@ -428,35 +431,40 @@
 		*rval1 = buf[0];
 	if (rval2)
 		*rval2 = buf[1];
+	return (USB_ERR_NORMAL_COMPLETION);
 }
 
-static void
+static usb_error_t
 uchcom_get_version(struct uchcom_softc *sc, uint8_t *rver)
 {
 	uint8_t buf[UCHCOM_INPUT_BUF_SIZE];
+	usb_error_t err;
 
-	uchcom_ctrl_read(sc, UCHCOM_REQ_GET_VERSION, 0, 0, buf, sizeof(buf));
+	err = uchcom_ctrl_read(sc, UCHCOM_REQ_GET_VERSION, 0, 0, buf, sizeof(buf));
+	if (err)
+		return (err);
 
 	if (rver)
 		*rver = buf[0];
+	return (USB_ERR_NORMAL_COMPLETION);
 }
 
-static void
+static usb_error_t
 uchcom_get_status(struct uchcom_softc *sc, uint8_t *rval)
 {
-	uchcom_read_reg(sc, UCHCOM_REG_STAT1, rval, UCHCOM_REG_STAT2, NULL);
+	return (uchcom_read_reg(sc, UCHCOM_REG_STAT1, rval, UCHCOM_REG_STAT2, NULL));
 }
 
-static void
+static usb_error_t
 uchcom_set_dtr_rts_10(struct uchcom_softc *sc, uint8_t val)
 {
-	uchcom_write_reg(sc, UCHCOM_REG_STAT1, val, UCHCOM_REG_STAT1, val);
+	return (uchcom_write_reg(sc, UCHCOM_REG_STAT1, val, UCHCOM_REG_STAT1, val));
 }
 
-static void
+static usb_error_t
 uchcom_set_dtr_rts_20(struct uchcom_softc *sc, uint8_t val)
 {
-	uchcom_ctrl_write(sc, UCHCOM_REQ_SET_DTRRTS, val, 0);
+	return (uchcom_ctrl_write(sc, UCHCOM_REQ_SET_DTRRTS, val, 0));
 }
 
 
@@ -464,10 +472,10 @@
  * middle layer
  */
 
-static void
+static usb_error_t
 uchcom_update_version(struct uchcom_softc *sc)
 {
-	uchcom_get_version(sc, &sc->sc_version);
+	return (uchcom_get_version(sc, &sc->sc_version));
 }
 
 static void
@@ -480,17 +488,21 @@
 	sc->sc_msr = (cur << 4) | ((sc->sc_msr >> 4) ^ cur);
 }
 
-static void
+static usb_error_t
 uchcom_update_status(struct uchcom_softc *sc)
 {
 	uint8_t cur;
+	usb_error_t err;
 
-	uchcom_get_status(sc, &cur);
+	err = uchcom_get_status(sc, &cur);
+	if (err)
+		return (err);
 	uchcom_convert_status(sc, cur);
+	return (USB_ERR_NORMAL_COMPLETION);
 }
 
 
-static void
+static usb_error_t
 uchcom_set_dtr_rts(struct uchcom_softc *sc)
 {
 	uint8_t val = 0;
@@ -501,9 +513,9 @@
 		val |= UCHCOM_RTS_MASK;
 
 	if (sc->sc_version < UCHCOM_VER_20)
-		uchcom_set_dtr_rts_10(sc, ~val);
+		return (uchcom_set_dtr_rts_10(sc, ~val));
 	else
-		uchcom_set_dtr_rts_20(sc, ~val);
+		return (uchcom_set_dtr_rts_20(sc, ~val));
 }
 
 static void
@@ -513,7 +525,9 @@
 	uint8_t brk1;
 	uint8_t brk2;
 
-	uchcom_read_reg(sc, UCHCOM_REG_BREAK1, &brk1, UCHCOM_REG_BREAK2, &brk2);
+	if (uchcom_read_reg(sc, UCHCOM_REG_BREAK1, &brk1,
+	    UCHCOM_REG_BREAK2, &brk2))
+		return;		/* DF-5421: device not answering */
 	if (onoff) {
 		/* on - clear bits */
 		brk1 &= ~UCHCOM_BRK1_MASK;
@@ -567,20 +581,23 @@
 	return (0);
 }
 
-static void
+static usb_error_t
 uchcom_set_baudrate(struct uchcom_softc *sc, uint32_t rate)
 {
 	struct uchcom_divider dv;
+	usb_error_t err;
 
 	if (uchcom_calc_divider_settings(&dv, rate))
-		return;
+		return (USB_ERR_INVAL);
 
-	uchcom_write_reg(sc,
+	err = uchcom_write_reg(sc,
 	    UCHCOM_REG_BPS_PRE, dv.dv_prescaler,
 	    UCHCOM_REG_BPS_DIV, dv.dv_div);
-	uchcom_write_reg(sc,
+	if (err)
+		return (err);
+	return (uchcom_write_reg(sc,
 	    UCHCOM_REG_BPS_MOD, dv.dv_mod,
-	    UCHCOM_REG_BPS_PAD, 0);
+	    UCHCOM_REG_BPS_PAD, 0));
 }
 
 /* ----------------------------------------------------------------------
@@ -605,7 +622,7 @@
 	DPRINTF("onoff = %d\n", onoff);
 
 	sc->sc_dtr = onoff;
-	uchcom_set_dtr_rts(sc);
+	(void)uchcom_set_dtr_rts(sc);
 }
 
 static void
@@ -616,7 +633,7 @@
 	DPRINTF("onoff = %d\n", onoff);
 
 	sc->sc_rts = onoff;
-	uchcom_set_dtr_rts(sc);
+	(void)uchcom_set_dtr_rts(sc);
 }
 
 static void
@@ -626,8 +643,9 @@
 
 	DPRINTF("\n");
 
-	uchcom_update_version(sc);
-	uchcom_update_status(sc);
+	if (uchcom_update_version(sc))
+		return;		/* DF-5421: abort on dead device */
+	(void)uchcom_update_status(sc);
 }
 
 static int
@@ -653,16 +671,30 @@
 {
 	struct uchcom_softc *sc = ucom->sc_parent;
 
-	uchcom_get_version(sc, 0);
-	uchcom_ctrl_write(sc, UCHCOM_REQ_RESET, 0, 0);
-	uchcom_set_baudrate(sc, t->c_ospeed);
-	uchcom_read_reg(sc, 0x18, 0, 0x25, 0);
-	uchcom_write_reg(sc, 0x18, 0x50, 0x25, 0x00);
-	uchcom_update_status(sc);
-	uchcom_ctrl_write(sc, UCHCOM_REQ_RESET, 0x501f, 0xd90a);
-	uchcom_set_baudrate(sc, t->c_ospeed);
-	uchcom_set_dtr_rts(sc);
-	uchcom_update_status(sc);
+	/*
+	 * DF-5421: every step is a synchronous 1000 ms-timeout EP0
+	 * request; abort on the first failure so an unresponsive
+	 * device costs one timeout, not twelve.
+	 */
+	if (uchcom_get_version(sc, 0))
+		return;
+	if (uchcom_ctrl_write(sc, UCHCOM_REQ_RESET, 0, 0))
+		return;
+	if (uchcom_set_baudrate(sc, t->c_ospeed))
+		return;
+	if (uchcom_read_reg(sc, 0x18, 0, 0x25, 0))
+		return;
+	if (uchcom_write_reg(sc, 0x18, 0x50, 0x25, 0x00))
+		return;
+	if (uchcom_update_status(sc))
+		return;
+	if (uchcom_ctrl_write(sc, UCHCOM_REQ_RESET, 0x501f, 0xd90a))
+		return;
+	if (uchcom_set_baudrate(sc, t->c_ospeed))
+		return;
+	if (uchcom_set_dtr_rts(sc))
+		return;
+	(void)uchcom_update_status(sc);
 }
 
 static void