svn commit: r189501 - head/sys/net

[Robert Watson]

Author: rwatson
Date: Sat Mar  7 22:17:44 2009
New Revision: 189501
URL: http://svn.freebsd.org/changeset/base/189501

Log:
  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

Modified:
  head/sys/net/bpf.c

Modified: head/sys/net/bpf.c
==============================================================================
--- head/sys/net/bpf.c	Sat Mar  7 22:05:58 2009	(r189500)
+++ head/sys/net/bpf.c	Sat Mar  7 22:17:44 2009	(r189501)
@@ -898,22 +898,28 @@ bpfwrite(struct cdev *dev, struct uio *u
 }
 
 /*
- * 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;