kern/59290: [PATCH] attaching more than one of the same usb if_* adapters crashes system

Daan Vreeken [PA4DAN] Danovitsch at Vitsch.net
Fri Nov 14 07:10:17 PST 2003


>Number:         59290
>Category:       kern
>Synopsis:       [PATCH] attaching more than one of the same usb if_* adapters crashes system
>Confidential:   no
>Severity:       critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Nov 14 07:10:13 PST 2003
>Closed-Date:
>Last-Modified:
>Originator:     Daan Vreeken [PA4DAN]
>Release:        FreeBSD 5.1-RELEASE i386
>Organization:
>Environment:
System: FreeBSD RaceBeest.Danovitsch.LAN 5.1-RELEASE FreeBSD 5.1-RELEASE #20: Mon Sep 1 16:39:56 CEST 2003 root at RaceBeest.Danovitsch.LAN:/home/src/sys/i386/compile/RaceBeest i386

>Description:
All drivers in /sys/dev/usb/if_*.c use a static variable called
qdat_[ifname] to store the interface pointer. When for example two "aue"
adapters are inserted to the USB port, the second will overwrite the first's
qdat-information.
The result is that now both adapters use the same interface pointer for
incoming packets (packets seem to come in only on the second interface now).
When the second interface is unplugged before the first one, qdat_aue still
contains the (invalid) pointer of the second interface. The first packet
coming in on the first interface now will crash the system.

>How-To-Repeat:
Insert aue adapter A and B into USB port.
Ping a random host on A's subnet and see the kernel complaining about
arp-packets coming in from the wrong interface.
Unplug adapter B.
Ping a random host on A's subnet and see how the system crashes.
>Fix:
Move qdat_* into the softc structure.
This solves the problem, but I'm not entirelly sure if the interface can be
unplugged before the qdat-pointer of a just received packet is used. I have
not experienced problems while brute force testing the patch, but I'm still
not entirelly sure...
If this can happen, keep the pr, dump the patch

--- diff begins here ---
diff -ur org-dev/usb/if_aue.c dev/usb/if_aue.c
--- org-dev/usb/if_aue.c	Fri Oct 31 19:32:06 2003
+++ dev/usb/if_aue.c	Thu Nov 13 14:35:00 2003
@@ -176,8 +176,6 @@
 };
 #define aue_lookup(v, p) ((const struct aue_type *)usb_lookup(aue_devs, v, p))
 
-Static struct usb_qdat aue_qdat;
-
 Static int aue_match(device_ptr_t);
 Static int aue_attach(device_ptr_t);
 Static int aue_detach(device_ptr_t);
@@ -773,8 +771,8 @@
 		USB_ATTACH_ERROR_RETURN;
 	}
 
-	aue_qdat.ifp = ifp;
-	aue_qdat.if_rxstart = aue_rxstart;
+	sc->aue_qdat.ifp = ifp;
+	sc->aue_qdat.if_rxstart = aue_rxstart;
 
 	/*
 	 * Call MI attach routine.
@@ -1040,7 +1038,7 @@
 	total_len -= (4 + ETHER_CRC_LEN);
 
 	ifp->if_ipackets++;
-	m->m_pkthdr.rcvif = (struct ifnet *)&aue_qdat;
+	m->m_pkthdr.rcvif = (struct ifnet *)&sc->aue_qdat;
 	m->m_pkthdr.len = m->m_len = total_len;
 
 	/* Put the packet on the special USB input queue. */
diff -ur org-dev/usb/if_auereg.h dev/usb/if_auereg.h
--- org-dev/usb/if_auereg.h	Sat Oct  4 23:41:01 2003
+++ dev/usb/if_auereg.h	Thu Nov 13 14:35:44 2003
@@ -248,6 +248,7 @@
 	u_int16_t		aue_flags;
 	char			aue_dying;
 	struct timeval		aue_rx_notice;
+	struct usb_qdat		aue_qdat;
 };
 
 #if 0
diff -ur org-dev/usb/if_axe.c dev/usb/if_axe.c
--- org-dev/usb/if_axe.c	Fri Oct 31 19:32:06 2003
+++ dev/usb/if_axe.c	Thu Nov 13 14:36:05 2003
@@ -118,8 +118,6 @@
 	{ 0, 0 }
 };
 
-Static struct usb_qdat axe_qdat;
-
 Static int axe_match(device_ptr_t);
 Static int axe_attach(device_ptr_t);
 Static int axe_detach(device_ptr_t);
@@ -520,8 +518,8 @@
 	ifp->if_baudrate = 10000000;
 	ifp->if_snd.ifq_maxlen = IFQ_MAXLEN;
 
-	axe_qdat.ifp = ifp;
-	axe_qdat.if_rxstart = axe_rxstart;
+	sc->axe_qdat.ifp = ifp;
+	sc->axe_qdat.if_rxstart = axe_rxstart;
 
 	if (mii_phy_probe(self, &sc->axe_miibus,
 	    axe_ifmedia_upd, axe_ifmedia_sts)) {
@@ -723,7 +721,7 @@
 	}
 
 	ifp->if_ipackets++;
-	m->m_pkthdr.rcvif = (struct ifnet *)&axe_qdat;
+	m->m_pkthdr.rcvif = (struct ifnet *)&sc->axe_qdat;
 	m->m_pkthdr.len = m->m_len = total_len;
 
 	/* Put the packet on the special USB input queue. */
diff -ur org-dev/usb/if_axereg.h dev/usb/if_axereg.h
--- org-dev/usb/if_axereg.h	Sun Jun 15 23:45:43 2003
+++ dev/usb/if_axereg.h	Thu Nov 13 14:36:20 2003
@@ -168,6 +168,7 @@
 	unsigned char		axe_ipgs[3];
 	unsigned char 		axe_phyaddrs[2];
 	struct timeval		axe_rx_notice;
+	struct usb_qdat		axe_qdat;
 };
 
 #if 0
diff -ur org-dev/usb/if_cue.c dev/usb/if_cue.c
--- org-dev/usb/if_cue.c	Fri Oct 31 19:32:06 2003
+++ dev/usb/if_cue.c	Thu Nov 13 14:36:37 2003
@@ -94,8 +94,6 @@
 	{ 0, 0 }
 };
 
-Static struct usb_qdat cue_qdat;
-
 Static int cue_match(device_ptr_t);
 Static int cue_attach(device_ptr_t);
 Static int cue_detach(device_ptr_t);
@@ -531,8 +529,8 @@
 	ifp->if_baudrate = 10000000;
 	ifp->if_snd.ifq_maxlen = IFQ_MAXLEN;
 
-	cue_qdat.ifp = ifp;
-	cue_qdat.if_rxstart = cue_rxstart;
+	sc->cue_qdat.ifp = ifp;
+	sc->cue_qdat.if_rxstart = cue_rxstart;
 
 	/*
 	 * Call MI attach routine.
@@ -747,7 +745,7 @@
 
 	ifp->if_ipackets++;
 	m_adj(m, sizeof(u_int16_t));
-	m->m_pkthdr.rcvif = (struct ifnet *)&cue_qdat;
+	m->m_pkthdr.rcvif = (struct ifnet *)&sc->cue_qdat;
 	m->m_pkthdr.len = m->m_len = total_len;
 
 	/* Put the packet on the special USB input queue. */
diff -ur org-dev/usb/if_cuereg.h dev/usb/if_cuereg.h
--- org-dev/usb/if_cuereg.h	Sat Oct  4 23:41:01 2003
+++ dev/usb/if_cuereg.h	Thu Nov 13 14:36:50 2003
@@ -182,6 +182,7 @@
 #endif
 	char			cue_dying;
 	struct timeval		cue_rx_notice;
+	struct usb_qdat		cue_qdat;
 };
 
 #if 0
diff -ur org-dev/usb/if_kue.c dev/usb/if_kue.c
--- org-dev/usb/if_kue.c	Fri Oct 31 19:32:06 2003
+++ dev/usb/if_kue.c	Thu Nov 13 14:37:27 2003
@@ -128,8 +128,6 @@
 	{ 0, 0 }
 };
 
-Static struct usb_qdat kue_qdat;
-
 Static int kue_match(device_ptr_t);
 Static int kue_attach(device_ptr_t);
 Static int kue_detach(device_ptr_t);
@@ -500,8 +498,8 @@
 	ifp->if_baudrate = 10000000;
 	ifp->if_snd.ifq_maxlen = IFQ_MAXLEN;
 
-	kue_qdat.ifp = ifp;
-	kue_qdat.if_rxstart = kue_rxstart;
+	sc->kue_qdat.ifp = ifp;
+	sc->kue_qdat.if_rxstart = kue_rxstart;
 
 	/*
 	 * Call MI attach routine.
@@ -721,7 +719,7 @@
 	}
 
 	ifp->if_ipackets++;
-	m->m_pkthdr.rcvif = (struct ifnet *)&kue_qdat;
+	m->m_pkthdr.rcvif = (struct ifnet *)&sc->kue_qdat;
 	m->m_pkthdr.len = m->m_len = total_len;
 
 	/* Put the packet on the special USB input queue. */
diff -ur org-dev/usb/if_kuereg.h dev/usb/if_kuereg.h
--- org-dev/usb/if_kuereg.h	Sat Oct  4 23:41:01 2003
+++ dev/usb/if_kuereg.h	Thu Nov 13 14:37:43 2003
@@ -174,6 +174,7 @@
 #endif
 	char			kue_dying;
 	struct timeval		kue_rx_notice;
+	struct usb_qdat		kue_qdat;
 };
 
 #if 0
diff -ur org-dev/usb/if_rue.c dev/usb/if_rue.c
--- org-dev/usb/if_rue.c	Fri Oct 31 19:32:06 2003
+++ dev/usb/if_rue.c	Thu Nov 13 14:37:54 2003
@@ -97,8 +97,6 @@
 	{ 0, 0 }
 };
 
-Static struct usb_qdat rue_qdat;
-
 Static int rue_match(device_ptr_t);
 Static int rue_attach(device_ptr_t);
 Static int rue_detach(device_ptr_t);
@@ -679,8 +677,8 @@
 		goto error1;
 	}
 
-	rue_qdat.ifp = ifp;
-	rue_qdat.if_rxstart = rue_rxstart;
+	sc->rue_qdat.ifp = ifp;
+	sc->rue_qdat.if_rxstart = rue_rxstart;
 
 	/* Call MI attach routine */
 #if __FreeBSD_version >= 500000
@@ -950,7 +948,7 @@
 	total_len -= ETHER_CRC_LEN;
 
 	ifp->if_ipackets++;
-	m->m_pkthdr.rcvif = (struct ifnet *)&rue_qdat;
+	m->m_pkthdr.rcvif = (struct ifnet *)&sc->rue_qdat;
 	m->m_pkthdr.len = m->m_len = total_len;
 
 	/* Put the packet on the special USB input queue. */
diff -ur org-dev/usb/if_ruereg.h dev/usb/if_ruereg.h
--- org-dev/usb/if_ruereg.h	Sat Oct  4 23:41:01 2003
+++ dev/usb/if_ruereg.h	Thu Nov 13 14:38:09 2003
@@ -228,6 +228,7 @@
 #endif
 	char			rue_dying;
 	struct timeval		rue_rx_notice;
+	struct usb_qdat		rue_qdat;
 };
 
 #if defined(__FreeBSD__)
>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list