Date: Tue, 28 May 2019 17:10:46 -0700 From: Gleb Smirnoff <glebius@freebsd.org> To: "Andrey V. Elsukov" <ae@freebsd.org>, kib@freebsd.org Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r348303 - head/sys/net Message-ID: <20190529001046.GC21836@FreeBSD.org> In-Reply-To: <201905271241.x4RCffTm047128@repo.freebsd.org> References: <201905271241.x4RCffTm047128@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Andrey,
I made a different change to mitigate this panic: don't clear the pointer.
--- a/FreeBSD/sys/net/bpf.c
+++ b/FreeBSD/sys/net/bpf.c
@@ -857,7 +857,6 @@ bpf_detachd_locked(struct bpf_d *d, bool detached_ifp)
/* Save bd_writer value */
error = d->bd_writer;
ifp = bp->bif_ifp;
- d->bd_bif = NULL;
if (detached_ifp) {
/*
* Notify descriptor as it's detached, so that any
Since every bpf_d holds a reference on bpf_if until delayed free happens,
the the bpf_if is going to be valid.
This allows not to use epoch_wait and run fully async. The patch above is
a minimal patch: with NULL assignment removed, several more pieces of code
can be removed in bpf.c
Of course your patch also is going to work, but what do you think:
are there any landmines with fully async approach?
On Mon, May 27, 2019 at 12:41:41PM +0000, Andrey V. Elsukov wrote:
A> Author: ae
A> Date: Mon May 27 12:41:41 2019
A> New Revision: 348303
A> URL: https://svnweb.freebsd.org/changeset/base/348303
A>
A> Log:
A> Fix possible NULL pointer dereference.
A>
A> bpf_mtap() can invoke catchpacket() for already detached descriptor.
A> And this can lead to NULL pointer dereference, since bd_bif pointer
A> was reset to NULL in bpf_detachd_locked(). To avoid this, use
A> NET_EPOCH_WAIT() when descriptor is removed from interface's descriptors
A> list. After the wait it is safe to modify descriptor's content.
A>
A> Submitted by: kib
A> Reported by: slavash
A> MFC after: 1 week
A>
A> Modified:
A> head/sys/net/bpf.c
A>
A> Modified: head/sys/net/bpf.c
A> ==============================================================================
A> --- head/sys/net/bpf.c Mon May 27 06:37:23 2019 (r348302)
A> +++ head/sys/net/bpf.c Mon May 27 12:41:41 2019 (r348303)
A> @@ -850,10 +850,15 @@ bpf_detachd_locked(struct bpf_d *d, bool detached_ifp)
A> /* Check if descriptor is attached */
A> if ((bp = d->bd_bif) == NULL)
A> return;
A> + /*
A> + * Remove d from the interface's descriptor list.
A> + * And wait until bpf_[m]tap*() will finish their possible work
A> + * with descriptor.
A> + */
A> + CK_LIST_REMOVE(d, bd_next);
A> + NET_EPOCH_WAIT();
A>
A> BPFD_LOCK(d);
A> - /* Remove d from the interface's descriptor list. */
A> - CK_LIST_REMOVE(d, bd_next);
A> /* Save bd_writer value */
A> error = d->bd_writer;
A> ifp = bp->bif_ifp;
A>
--
Gleb Smirnoff
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190529001046.GC21836>