Date: Sat, 7 Mar 2009 22:17:44 +0000 (UTC) From: Robert Watson <rwatson@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r189501 - head/sys/net Message-ID: <200903072217.n27MHiw3064313@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200903072217.n27MHiw3064313>