Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 10 Jan 2012 00:48:30 +0000 (UTC)
From:      Lawrence Stewart <lstewart@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r229898 - head/sys/net
Message-ID:  <201201100048.q0A0mUmu071589@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: lstewart
Date: Tue Jan 10 00:48:29 2012
New Revision: 229898
URL: http://svn.freebsd.org/changeset/base/229898

Log:
  Consumers of bpfdetach() expect it to remove all bpf_if structs from the
  bpf_iflist list which reference the specified ifnet. The existing implementation
  only removes the first matching bpf_if found in the list, effectively leaking
  list entries if an ifnet has been bpfattach()ed multiple times with different
  DLTs.
  
  Fix the leak by performing the detach logic in a loop, stopping when all bpf_if
  structs referencing the specified ifnet have been detached and removed from the
  bpf_iflist list.
  
  Whilst here, also:
  
  - Remove the unnecessary "bp->bif_ifp == NULL" check, as a bpf_if should never
    exist in the list with a NULL ifnet pointer.
  
  - Except when INVARIANTS is in the kernel config, silently ignore the case where
    no bpf_if referencing the specified ifnet is found, as it is harmless and does
    not require user attention.
  
  Reviewed by:	csjp
  MFC after:	1 week

Modified:
  head/sys/net/bpf.c

Modified: head/sys/net/bpf.c
==============================================================================
--- head/sys/net/bpf.c	Tue Jan 10 00:35:25 2012	(r229897)
+++ head/sys/net/bpf.c	Tue Jan 10 00:48:29 2012	(r229898)
@@ -2253,33 +2253,42 @@ bpfdetach(struct ifnet *ifp)
 {
 	struct bpf_if	*bp;
 	struct bpf_d	*d;
+#ifdef INVARIANTS
+	int ndetached;
 
-	/* Locate BPF interface information */
-	mtx_lock(&bpf_mtx);
-	LIST_FOREACH(bp, &bpf_iflist, bif_next) {
-		if (ifp == bp->bif_ifp)
-			break;
-	}
+	ndetached = 0;
+#endif
 
-	/* Interface wasn't attached */
-	if ((bp == NULL) || (bp->bif_ifp == NULL)) {
+	/* Find all bpf_if struct's which reference ifp and detach them. */
+	do {
+		mtx_lock(&bpf_mtx);
+		LIST_FOREACH(bp, &bpf_iflist, bif_next) {
+			if (ifp == bp->bif_ifp)
+				break;
+		}
+		if (bp != NULL)
+			LIST_REMOVE(bp, bif_next);
 		mtx_unlock(&bpf_mtx);
-		printf("bpfdetach: %s was not attached\n", ifp->if_xname);
-		return;
-	}
-
-	LIST_REMOVE(bp, bif_next);
-	mtx_unlock(&bpf_mtx);
 
-	while ((d = LIST_FIRST(&bp->bif_dlist)) != NULL) {
-		bpf_detachd(d);
-		BPFD_LOCK(d);
-		bpf_wakeup(d);
-		BPFD_UNLOCK(d);
-	}
+		if (bp != NULL) {
+#ifdef INVARIANTS
+			ndetached++;
+#endif
+			while ((d = LIST_FIRST(&bp->bif_dlist)) != NULL) {
+				bpf_detachd(d);
+				BPFD_LOCK(d);
+				bpf_wakeup(d);
+				BPFD_UNLOCK(d);
+			}
+			mtx_destroy(&bp->bif_mtx);
+			free(bp, M_BPF);
+		}
+	} while (bp != NULL);
 
-	mtx_destroy(&bp->bif_mtx);
-	free(bp, M_BPF);
+#ifdef INVARIANTS
+	if (ndetached == 0)
+		printf("bpfdetach: %s was not attached\n", ifp->if_xname);
+#endif
 }
 
 /*

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201201100048.q0A0mUmu071589>