When resetting a BPF descriptor, properly check that zero-copy buffers

are not currently owned by userspace before clearing or rotating them.

Otherwise we may not play by the rules of the shared memory protocol,
potentially corrupting packet data or causing userspace applications
that are playing by the rules to spin due to being notified that a
buffer is complete but the shared memory header not reflecting that.

This behavior was seen with pflogd by a number of reporters; note that
this fix is not sufficient to get pflogd properly working with
zero-copy BPF, due to pflogd opening the BPF device before forking,
leading to the shared memory buffer not being propery inherited in the
privilege-separated child.  We're still deciding how to fix that
problem.

This change exposes buffer-model specific strategy information in
reset_d(), which will be fixed at a later date once we've decided how
best to improve the BPF buffer abstraction.

Reviewed by:	csjp
Reported by:	keramida

1 files changed, 11 insertions, 5 deletions

diff --git a/sys/net/bpf.c b/sys/net/bpf.c
index 21d4969..f8f605b 100644
--- a/sys/net/bpf.c
+++ b/sys/net/bpf.c
@@ -898,22 +898,28 @@ bpfwrite(struct cdev *dev, struct uio *uio, int ioflag)
 }
 
 /*
- * Reset a descriptor by flushing its packet buffer and clearing the
- * receive and drop counts.
+ * Reset a descriptor by flushing its packet buffer and clearing the receive
+ * and drop counts.  This is doable for kernel-only buffers, but with
+ * zero-copy buffers, we can't write to (or rotate) buffers that are
+ * currently owned by userspace.  It would be nice if we could encapsulate
+ * this logic in the buffer code rather than here.
  */
 static void
 reset_d(struct bpf_d *d)
 {
 
 	mtx_assert(&d->bd_mtx, MA_OWNED);
-	if (d->bd_hbuf) {
+
+	if ((d->bd_hbuf != NULL) &&
+	    (d->bd_bufmode != BPF_BUFMODE_ZBUF || bpf_canfreebuf(d))) {
 		/* Free the hold buffer. */
 		d->bd_fbuf = d->bd_hbuf;
 		d->bd_hbuf = NULL;
+		d->bd_hlen = 0;
 		bpf_buf_reclaimed(d);
 	}
-	d->bd_slen = 0;
-	d->bd_hlen = 0;
+	if (bpf_canwritebuf(d))
+		d->bd_slen = 0;
 	d->bd_rcount = 0;
 	d->bd_dcount = 0;
 	d->bd_fcount = 0;