Check the validity of the `qtd' pointer after each interrupt case
authorvisa <visa@openbsd.org>
Tue, 20 Jun 2017 15:49:00 +0000 (15:49 +0000)
committervisa <visa@openbsd.org>
Tue, 20 Jun 2017 15:49:00 +0000 (15:49 +0000)
and bail out if the pointer has become invalid. This prevents
use-after-free memory accesses that corrupted the dwc2qtd pool.

This fix improves stability on the Raspberry Pi 3.

From Linux commit dc8730846948e517169f630826cd2c97615f5ee8
OK kettenis@

sys/dev/usb/dwc2/dwc2_hcdintr.c

index 34a0d9d..3735c81 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: dwc2_hcdintr.c,v 1.7 2017/02/12 17:41:17 visa Exp $   */
+/*     $OpenBSD: dwc2_hcdintr.c,v 1.8 2017/06/20 15:49:00 visa Exp $   */
 /*     $NetBSD: dwc2_hcdintr.c,v 1.11 2014/11/24 10:14:14 skrll Exp $  */
 
 /*
@@ -1951,6 +1951,20 @@ STATIC void dwc2_hc_chhltd_intr(struct dwc2_hsotg *hsotg,
        }
 }
 
+/*
+ * Check if the given qtd is still the top of the list (and thus valid).
+ *
+ * If dwc2_hcd_qtd_unlink_and_free() has been called since we grabbed
+ * the qtd from the top of the list, this will return false (otherwise true).
+ */
+STATIC bool dwc2_check_qtd_still_ok(struct dwc2_qtd *qtd, struct dwc2_qh *qh)
+{
+       if (!qh)
+               return false;
+
+       return (TAILQ_FIRST(&qh->qtd_list) == qtd);
+}
+
 /* Handles interrupt for a specific Host Channel */
 STATIC void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, int chnum)
 {
@@ -2032,27 +2046,59 @@ STATIC void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, int chnum)
                 */
                hcint &= ~HCINTMSK_NYET;
        }
-       if (hcint & HCINTMSK_CHHLTD)
+
+       if (hcint & HCINTMSK_CHHLTD) {
                dwc2_hc_chhltd_intr(hsotg, chan, chnum, qtd);
-       if (hcint & HCINTMSK_AHBERR)
+               if (!dwc2_check_qtd_still_ok(qtd, chan->qh))
+                       goto exit;
+       }
+       if (hcint & HCINTMSK_AHBERR) {
                dwc2_hc_ahberr_intr(hsotg, chan, chnum, qtd);
-       if (hcint & HCINTMSK_STALL)
+               if (!dwc2_check_qtd_still_ok(qtd, chan->qh))
+                       goto exit;
+       }
+       if (hcint & HCINTMSK_STALL) {
                dwc2_hc_stall_intr(hsotg, chan, chnum, qtd);
-       if (hcint & HCINTMSK_NAK)
+               if (!dwc2_check_qtd_still_ok(qtd, chan->qh))
+                       goto exit;
+       }
+       if (hcint & HCINTMSK_NAK) {
                dwc2_hc_nak_intr(hsotg, chan, chnum, qtd);
-       if (hcint & HCINTMSK_ACK)
+               if (!dwc2_check_qtd_still_ok(qtd, chan->qh))
+                       goto exit;
+       }
+       if (hcint & HCINTMSK_ACK) {
                dwc2_hc_ack_intr(hsotg, chan, chnum, qtd);
-       if (hcint & HCINTMSK_NYET)
+               if (!dwc2_check_qtd_still_ok(qtd, chan->qh))
+                       goto exit;
+       }
+       if (hcint & HCINTMSK_NYET) {
                dwc2_hc_nyet_intr(hsotg, chan, chnum, qtd);
-       if (hcint & HCINTMSK_XACTERR)
+               if (!dwc2_check_qtd_still_ok(qtd, chan->qh))
+                       goto exit;
+       }
+       if (hcint & HCINTMSK_XACTERR) {
                dwc2_hc_xacterr_intr(hsotg, chan, chnum, qtd);
-       if (hcint & HCINTMSK_BBLERR)
+               if (!dwc2_check_qtd_still_ok(qtd, chan->qh))
+                       goto exit;
+       }
+       if (hcint & HCINTMSK_BBLERR) {
                dwc2_hc_babble_intr(hsotg, chan, chnum, qtd);
-       if (hcint & HCINTMSK_FRMOVRUN)
+               if (!dwc2_check_qtd_still_ok(qtd, chan->qh))
+                       goto exit;
+       }
+       if (hcint & HCINTMSK_FRMOVRUN) {
                dwc2_hc_frmovrun_intr(hsotg, chan, chnum, qtd);
-       if (hcint & HCINTMSK_DATATGLERR)
+               if (!dwc2_check_qtd_still_ok(qtd, chan->qh))
+                       goto exit;
+       }
+       if (hcint & HCINTMSK_DATATGLERR) {
                dwc2_hc_datatglerr_intr(hsotg, chan, chnum, qtd);
+               if (!dwc2_check_qtd_still_ok(qtd, chan->qh))
+                       goto exit;
+       }
 
+exit:
        chan->hcint = 0;
 }