Change 61226 by rwatson@rwatson_paprika on 2004/09/09 02:55:04

	Reformulate bpf_attachd() to acquire the BIF_LOCK() as well as
	BPFD_LOCK() when removing a descriptor from an interface
	descriptor list.  Hold both over the operation, and improve
	maintenance of the invariant that you can't find partially
	connected descriptors on an active interface descriptor list.
	This may close a race causing kernel null pointer dereferences
	during heavy packet receipt/send and tcpdump being killed.

Affected files ...

... //depot/user/rwatson/netperf/sys/net/bpf.c#21 edit

Differences ...

==== //depot/user/rwatson/netperf/sys/net/bpf.c#21 (text+ko) ====

@@ -271,17 +271,42 @@
 {
 	int error;
 	struct bpf_if *bp;
+	struct ifnet *ifp;
 
-	/* XXX locking */
+	/*
+	 * XXXRW: Watch out for bpfdetach() in the other direction: can
+	 * d->bd_bif become NULL here?
+	 */
 	bp = d->bd_bif;
+	BPFIF_LOCK(bp);
+	BPFD_LOCK(d);
+	ifp = d->bd_bif->bif_ifp;
+
+	/*
+	 * Remove d from the interface's descriptor list.
+	 */
+	LIST_REMOVE(d, bd_next);
+
+	/*
+	 * Let the driver know that there are no more listeners.
+	 */
+	if (LIST_EMPTY(&bp->bif_dlist))
+		*bp->bif_driverp = NULL;
+
 	d->bd_bif = NULL;
+	BPFD_UNLOCK(d);
+	BPFIF_UNLOCK(bp);
+
 	/*
 	 * Check if this descriptor had requested promiscuous mode.
 	 * If so, turn it off.
+	 *
+	 * XXXRW: Note, ifp might be stale here if the ifnet has been
+	 * removed via bpfdetach().
 	 */
 	if (d->bd_promisc) {
 		d->bd_promisc = 0;
-		error = ifpromisc(bp->bif_ifp, 0);
+		error = ifpromisc(ifp, 0);
 		if (error != 0 && error != ENXIO) {
 			/*
 			 * ENXIO can happen if a pccard is unplugged
@@ -293,15 +318,6 @@
 				"bpf_detach: ifpromisc failed (%d)\n", error);
 		}
 	}
-	/* Remove d from the interface's descriptor list. */
-	BPFIF_LOCK(bp);
-	LIST_REMOVE(d, bd_next);
-	if (LIST_EMPTY(&bp->bif_dlist))
-		/*
-		 * Let the driver know that there are no more listeners.
-		 */
-		*bp->bif_driverp = NULL;
-	BPFIF_UNLOCK(bp);
 }
 
 /*