Attachment 221506 Details for Bug 252608 – Patch for second issue - please test

[patch] Patch for second issue - please test

if.diff (text/plain), 4.89 KB, created by Hans Petter Selasky on 2021-01-12 19:47:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Hans Petter Selasky

Created: 2021-01-12 19:47:33 UTC

Size: 4.89 KB

patch

obsolete

>commit f1494986591aa58d9ef5ecedc1954fef7c6c8609
>Author: Hans Petter Selasky <hselasky@FreeBSD.org>
>Date:   Tue Jan 12 18:51:09 2021 +0100
>
>    Fix for use-after-free by if_ioctl callback in USB drivers.
>    
>    PR: 252608
>    MFC after: 1 week
>    Sponsored by: Mellanox Technologies // NVIDIA Networking
>
>diff --git a/sys/dev/usb/net/if_usie.c b/sys/dev/usb/net/if_usie.c
>index 8c64ad71be4..14ca9cc5802 100644
>--- a/sys/dev/usb/net/if_usie.c
>+++ b/sys/dev/usb/net/if_usie.c
>@@ -483,6 +483,7 @@ usie_detach(device_t self)
> 		usbd_transfer_unsetup(sc->sc_if_xfer, USIE_IF_N_XFER);
> 		bpfdetach(sc->sc_ifp);
> 		if_detach(sc->sc_ifp);
>+		if_slow_drain(sc->sc_ifp);
> 		if_free(sc->sc_ifp);
> 		sc->sc_ifp = NULL;
> 	}
>diff --git a/sys/dev/usb/net/uhso.c b/sys/dev/usb/net/uhso.c
>index c72fa8d7a36..e3854eccd56 100644
>--- a/sys/dev/usb/net/uhso.c
>+++ b/sys/dev/usb/net/uhso.c
>@@ -693,8 +693,9 @@ uhso_detach(device_t self)
> 		uhso_if_stop(sc);
> 		bpfdetach(sc->sc_ifp);
> 		if_detach(sc->sc_ifp);
>-		if_free(sc->sc_ifp);
> 		mtx_unlock(&sc->sc_mtx);
>+		if_slow_drain(sc->sc_ifp);
>+		if_free(sc->sc_ifp);
> 		usbd_transfer_unsetup(sc->sc_if_xfer, UHSO_IFNET_MAX);
> 	}
> 
>diff --git a/sys/dev/usb/net/usb_ethernet.c b/sys/dev/usb/net/usb_ethernet.c
>index e1eb2c247b5..da76e8f59e9 100644
>--- a/sys/dev/usb/net/usb_ethernet.c
>+++ b/sys/dev/usb/net/usb_ethernet.c
>@@ -292,6 +292,7 @@ ue_attach_post_task(struct usb_proc_msg *_task)
> 	/* free unit */
> 	free_unr(ueunit, ue->ue_unit);
> 	if (ue->ue_ifp != NULL) {
>+		if_slow_drain(ue->ue_ifp);
> 		if_free(ue->ue_ifp);
> 		ue->ue_ifp = NULL;
> 	}
>@@ -311,6 +312,9 @@ uether_ifdetach(struct usb_ether *ue)
> 	ifp = ue->ue_ifp;
> 
> 	if (ifp != NULL) {
>+		/* drain all IOCTLs */
>+		if_slow_drain(ifp);
>+
> 		/* we are not running any more */
> 		UE_LOCK(ue);
> 		ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
>diff --git a/sys/dev/usb/usb_pf.c b/sys/dev/usb/usb_pf.c
>index 6ccb5ebbc62..481aff704fe 100644
>--- a/sys/dev/usb/usb_pf.c
>+++ b/sys/dev/usb/usb_pf.c
>@@ -232,6 +232,7 @@ usbpf_clone_destroy(struct if_clone *ifc, struct ifnet *ifp)
> 	USB_BUS_UNLOCK(ubus);
> 	bpfdetach(ifp);
> 	if_detach(ifp);
>+	if_slow_drain(ifp);
> 	if_free(ifp);
> 	ifc_free_unit(ifc, unit);
> 
>diff --git a/sys/net/if.c b/sys/net/if.c
>index 68ed4cf65dc..4fd38db1df5 100644
>--- a/sys/net/if.c
>+++ b/sys/net/if.c
>@@ -645,6 +645,8 @@ if_alloc_domain(u_char type, int numa_domain)
> 	ifq_init(&ifp->if_snd, ifp);
> 
> 	refcount_init(&ifp->if_refcount, 1);	/* Index reference. */
>+	refcount_init(&ifp->if_slowref, 1);
>+
> 	for (int i = 0; i < IFCOUNTERS; i++)
> 		ifp->if_counters[i] = counter_u64_alloc(M_WAITOK);
> 	ifp->if_get_counter = if_get_counter_default;
>@@ -753,6 +755,33 @@ if_rele(struct ifnet *ifp)
> 	NET_EPOCH_CALL(if_destroy, &ifp->if_epoch_ctx);
> }
> 
>+/*
>+ * Keep track of slow path configuration events.
>+ * Returns true on success and false on failure.
>+ */
>+bool
>+if_slow_ref(struct ifnet *ifp)
>+{
>+	return (refcount_acquire_if_not_zero(&ifp->if_slowref));
>+}
>+
>+void
>+if_slow_drain(struct ifnet *ifp)
>+{
>+	if (refcount_release(&ifp->if_slowref))
>+		return;
>+
>+	while (refcount_load(&ifp->if_slowref) != 0)
>+		pause("W", hz);
>+}
>+
>+void
>+if_slow_unref(struct ifnet *ifp)
>+{
>+	if (refcount_release(&ifp->if_slowref))
>+		return;
>+}
>+
> void
> ifq_init(struct ifaltq *ifq, struct ifnet *ifp)
> {
>@@ -2459,8 +2488,8 @@ ifr_data_get_ptr(void *ifrp)
> /*
>  * Hardware specific interface ioctls.
>  */
>-int
>-ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
>+static inline int
>+ifhwioctl_sub(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
> {
> 	struct ifreq *ifr;
> 	int error = 0, do_ifup = 0;
>@@ -2887,6 +2916,19 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
> 	return (error);
> }
> 
>+int
>+ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
>+{
>+	int error;
>+
>+	if (if_slow_ref(ifp) == false)
>+		return (ENXIO);
>+	error = ifhwioctl_sub(cmd, ifp, data, td);
>+	if_slow_unref(ifp);
>+	return (error);
>+}
>+
>+
> #ifdef COMPAT_FREEBSD32
> struct ifconf32 {
> 	int32_t	ifc_len;
>diff --git a/sys/net/if_var.h b/sys/net/if_var.h
>index beb9596895e..0fda713cf40 100644
>--- a/sys/net/if_var.h
>+++ b/sys/net/if_var.h
>@@ -312,6 +312,7 @@ struct ifnet {
> 	void	*if_linkmib;		/* link-type-specific MIB data */
> 	size_t	if_linkmiblen;		/* length of above data */
> 	u_int	if_refcount;		/* reference count */
>+  	u_int	if_slowref;		/* reference count (slow path) */
> 
> 	/* These fields are shared with struct if_data. */
> 	uint8_t		if_type;	/* ethernet, tokenring, etc */
>@@ -658,6 +659,9 @@ void	if_link_state_change(struct ifnet *, int);
> int	if_printf(struct ifnet *, const char *, ...) __printflike(2, 3);
> void	if_ref(struct ifnet *);
> void	if_rele(struct ifnet *);
>+bool	if_slow_ref(struct ifnet *) __result_use_check;
>+void	if_slow_drain(struct ifnet *);
>+void	if_slow_unref(struct ifnet *);
> int	if_setlladdr(struct ifnet *, const u_char *, int);
> int	if_tunnel_check_nesting(struct ifnet *, struct mbuf *, uint32_t, int);
> void	if_up(struct ifnet *);

Actions: View | Diff

Attachments on bug 252608: 221491 | 221501 | 221506