From owner-dev-commits-src-main@freebsd.org Tue Jan 12 17:33:42 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 761A04E0166; Tue, 12 Jan 2021 17:33:42 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DFd3B2yNfz3sPN; Tue, 12 Jan 2021 17:33:42 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 58722BF4; Tue, 12 Jan 2021 17:33:42 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 10CHXgYX052487; Tue, 12 Jan 2021 17:33:42 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 10CHXg1q052486; Tue, 12 Jan 2021 17:33:42 GMT (envelope-from git) Date: Tue, 12 Jan 2021 17:33:42 GMT Message-Id: <202101121733.10CHXg1q052486@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Hans Petter Selasky Subject: git: 6e5baec33c10 - main - Fix for use-after-free in if_ure(4) driver. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: hselasky X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 6e5baec33c1032f4fbf713d601a34b4658b918a2 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jan 2021 17:33:42 -0000 The branch main has been updated by hselasky: URL: https://cgit.FreeBSD.org/src/commit/?id=6e5baec33c1032f4fbf713d601a34b4658b918a2 commit 6e5baec33c1032f4fbf713d601a34b4658b918a2 Author: Hans Petter Selasky AuthorDate: 2021-01-12 13:13:14 +0000 Commit: Hans Petter Selasky 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;