git: 6e5baec33c10 - main - Fix for use-after-free in if_ure(4) driver.

[Hans Petter Selasky]

The branch main has been updated by hselasky:

URL: https://cgit.FreeBSD.org/src/commit/?id=6e5baec33c1032f4fbf713d601a34b4658b918a2

commit 6e5baec33c1032f4fbf713d601a34b4658b918a2
Author:     Hans Petter Selasky <hselasky at FreeBSD.org>
AuthorDate: 2021-01-12 13:13:14 +0000
Commit:     Hans Petter Selasky <hselasky at FreeBSD.org>
CommitDate: 2021-01-12 16:57:58 +0000

    Fix for use-after-free in if_ure(4) driver.
    
    When detaching the if_ure(4) driver, the TX active USB transfer array may
    point to freed USB transfers. Given that the number of USB transfers is
    very low, simply start all transfers every time there is a packet to
    keep safe from use-after-free.
    
    PR: 252608
    MFC after: 1 week
    Sponsored by: Mellanox Technologies // NVIDIA Networking
---
 sys/dev/usb/net/if_ure.c    | 49 ++++-----------------------------------------
 sys/dev/usb/net/if_urereg.h |  9 ---------
 2 files changed, 4 insertions(+), 54 deletions(-)

diff --git a/sys/dev/usb/net/if_ure.c b/sys/dev/usb/net/if_ure.c
index 9d43b9a59a44..30fcee59cce3 100644
--- a/sys/dev/usb/net/if_ure.c
+++ b/sys/dev/usb/net/if_ure.c
@@ -581,13 +581,6 @@ ure_attach(device_t dev)
 		goto detach;
 	}
 
-	/* Mark all TX transfers as available */
-	for (int i = 0; i < URE_N_TRANSFER; i++) {
-		sc->sc_txavail[i] = sc->sc_tx_xfer[i];
-		DEVPRINTF(dev, "sc_txavail[%d] = %p\n", i, sc->sc_txavail[i]);
-	}
-	sc->sc_txpos = 0;
-
 	ue->ue_sc = sc;
 	ue->ue_dev = dev;
 	ue->ue_udev = uaa->device;
@@ -870,16 +863,6 @@ pkterror:
 
 		usbd_transfer_submit(xfer);
 
-		KASSERT(sc->sc_txpos >= 0 && sc->sc_txpos <= URE_N_TRANSFER,
-		    ("sc_txpos invalid: %d", sc->sc_txpos));
-		if (sc->sc_txpos < URE_N_TRANSFER &&
-		    !IFQ_DRV_IS_EMPTY(&ifp->if_snd)) {
-			xfer = sc->sc_txavail[sc->sc_txpos++];
-			usbd_transfer_start(xfer);
-		}
-
-		if (sc->sc_txpos == URE_N_TRANSFER)
-			ifp->if_drv_flags |= IFF_DRV_OACTIVE;
 		return;
 
 	default:			/* Error */
@@ -900,11 +883,6 @@ pkterror:
 			goto tr_setup;
 		}
 	}
-
-	KASSERT(sc->sc_txpos > 0 && sc->sc_txpos <= URE_N_TRANSFER, ("sc_txpos invalid: %d", sc->sc_txpos));
-	sc->sc_txavail[(--(sc->sc_txpos))] = xfer;
-	if (sc->sc_txpos < URE_N_TRANSFER)
-		ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
 }
 
 static void
@@ -1110,10 +1088,7 @@ ure_tick(struct usb_ether *ue)
 
 	URE_LOCK_ASSERT(sc, MA_OWNED);
 
-	KASSERT(sc->sc_txpos >= 0 && sc->sc_txpos <= URE_N_TRANSFER, ("sc_txpos invalid: %d", sc->sc_txpos));
 	(void)ifp;
-	DEVPRINTFN(13, sc->sc_ue.ue_dev,
-	    "sc_txpos: %d, oactive: %d\n", sc->sc_txpos, !!(ifp->if_drv_flags & IFF_DRV_OACTIVE));
 	for (int i = 0; i < URE_N_TRANSFER; i++)
 		DEVPRINTFN(13, sc->sc_ue.ue_dev,
 		    "rx[%d] = %d\n", i, USB_GET_STATE(sc->sc_rx_xfer[i]));
@@ -1190,34 +1165,18 @@ static void
 ure_start(struct usb_ether *ue)
 {
 	struct ure_softc *sc = uether_getsc(ue);
-	struct usb_xfer *xfer;
-	struct ifnet *ifp;
+	unsigned i;
 
 	URE_LOCK_ASSERT(sc, MA_OWNED);
 
 	if (!sc->sc_rxstarted) {
 		sc->sc_rxstarted = 1;
-		for (int i = 0; i < URE_N_TRANSFER; i++)
+		for (i = 0; i != URE_N_TRANSFER; i++)
 			usbd_transfer_start(sc->sc_rx_xfer[i]);
 	}
 
-	/*
-	 * start the USB transfers, if not already started:
-	 */
-	if (sc->sc_txpos == URE_N_TRANSFER) {
-		ifp = uether_getifp(&sc->sc_ue);
-
-		ifp->if_drv_flags |= IFF_DRV_OACTIVE;
-		return;
-	}
-
-	KASSERT(sc->sc_txpos >= 0 && sc->sc_txpos < URE_N_TRANSFER, ("sc_txpos invalid: %d", sc->sc_txpos));
-	xfer = sc->sc_txavail[sc->sc_txpos++];
-	if (sc->sc_txpos == URE_N_TRANSFER) {
-		ifp = uether_getifp(&sc->sc_ue);
-		ifp->if_drv_flags |= IFF_DRV_OACTIVE;
-	}
-	usbd_transfer_start(xfer);
+	for (i = 0; i != URE_N_TRANSFER; i++)
+		usbd_transfer_start(sc->sc_tx_xfer[i]);
 }
 
 static void
diff --git a/sys/dev/usb/net/if_urereg.h b/sys/dev/usb/net/if_urereg.h
index 2eb3de4d48e8..6b6f99434313 100644
--- a/sys/dev/usb/net/if_urereg.h
+++ b/sys/dev/usb/net/if_urereg.h
@@ -445,15 +445,6 @@ struct ure_softc {
 
 	int			sc_rxstarted;
 
-	struct usb_xfer		*sc_txavail[URE_N_TRANSFER];
-	/*
-	 * Position of next available xfer for TX.  If
-	 * sc_txpos == URE_N_TRANSFER, no tx xfer's are available.
-	 * Pop xfer:  sc->sc_txavail[sc->sc_txpos++]
-	 * Push xfer: sc->sc_txavail[(--(sc->sc_txpos))] = xfer
-	 */
-	int			sc_txpos;
-
 	int			sc_phyno;
 
 	u_int			sc_flags;