From 9ce40d321dd561ab74fad5881985fd23bb5b3c21 Mon Sep 17 00:00:00 2001 From: Kristof Provost Date: Wed, 16 Aug 2017 19:40:07 +0000 Subject: [PATCH] bpf: Fix incorrect cleanup Cleaning up a bpf_if is a two stage process. We first move it to the bpf_freelist (in bpfdetach()) and only later do we actually free it (in bpf_ifdetach()). We cannot set the ifp->if_bpf to NULL from bpf_ifdetach() because it's possible that the ifnet has already gone away, or that it has been assigned a new bpf_if. This can lead to a struct ifnet which is up, but has if_bpf set to NULL, which will panic when we try to send the next packet. Keep track of the pointer to the bpf_if (because it's not always ifp->if_bpf), and NULL it immediately in bpfdetach(). PR: 213896 MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D11782 --- sys/net/bpf.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/sys/net/bpf.c b/sys/net/bpf.c index 2dca1b444a45..b176856cf352 100644 --- a/sys/net/bpf.c +++ b/sys/net/bpf.c @@ -106,6 +106,7 @@ struct bpf_if { struct rwlock bif_lock; /* interface lock */ LIST_HEAD(, bpf_d) bif_wlist; /* writer-only list */ int bif_flags; /* Interface flags */ + struct bpf_if **bif_bpf; /* Pointer to pointer to us */ }; CTASSERT(offsetof(struct bpf_if, bif_ext) == 0); @@ -2563,6 +2564,7 @@ bpfattach2(struct ifnet *ifp, u_int dlt, u_int hdrlen, struct bpf_if **driverp) bp->bif_dlt = dlt; rw_init(&bp->bif_lock, "bpf interface lock"); KASSERT(*driverp == NULL, ("bpfattach2: driverp already initialized")); + bp->bif_bpf = driverp; *driverp = bp; BPF_LOCK(); @@ -2633,6 +2635,7 @@ bpfdetach(struct ifnet *ifp) */ BPFIF_WLOCK(bp); bp->bif_flags |= BPFIF_FLAG_DYING; + *bp->bif_bpf = NULL; BPFIF_WUNLOCK(bp); CTR4(KTR_NET, "%s: sheduling free for encap %d (%p) for if %p", @@ -2702,13 +2705,6 @@ bpf_ifdetach(void *arg __unused, struct ifnet *ifp) nmatched++; } BPF_UNLOCK(); - - /* - * Note that we cannot zero other pointers to - * custom DLTs possibly used by given interface. - */ - if (nmatched != 0) - ifp->if_bpf = NULL; } /*