--- 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. --- --- - 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: ? --- MFC after: 1 week --- Index: sys/net/bpf.c =================================================================== --- sys/net/bpf.c (revision 229165) +++ sys/net/bpf.c (working copy) @@ -2254,32 +2254,28 @@ bpfdetach(struct ifnet *ifp) struct bpf_if *bp; struct bpf_d *d; - /* Locate BPF interface information */ - mtx_lock(&bpf_mtx); - LIST_FOREACH(bp, &bpf_iflist, bif_next) { - if (ifp == bp->bif_ifp) - break; - } - - /* 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); - } - - mtx_destroy(&bp->bif_mtx); - free(bp, M_BPF); + if (bp != NULL) { + 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); } /*