Still working this problem I've previously mentioned, and my working theory now is a race between catchpacket() and this code in bpfread():
/* * At this point, we know we have something in the hold slot. */ BPFD_UNLOCK(d); /* * Move data from hold buffer into user space. * We know the entire buffer is transferred since * we checked above that the read buffer is bpf_bufsize bytes. * * XXXRW: More synchronization needed here: what if a second thread * issues a read on the same fd at the same time? Don't want this * getting invalidated. */ error = bpf_uiomove(d, d->bd_hbuf, d->bd_hlen, uio); BPFD_LOCK(d); d->bd_fbuf = d->bd_hbuf; d->bd_hbuf = NULL; d->bd_hlen = 0; bpf_buf_reclaimed(d); BPFD_UNLOCK(d); Assuming it's unwise to hold the descriptor lock over the uiomove() call, it seems we at least need to check to make sure bd_hbuf is still valid: @@ -809,10 +948,15 @@ bpfread(struct cdev *dev, struct uio *uio, int iof error = bpf_uiomove(d, d->bd_hbuf, d->bd_hlen, uio); BPFD_LOCK(d); - d->bd_fbuf = d->bd_hbuf; - d->bd_hbuf = NULL; - d->bd_hlen = 0; - bpf_buf_reclaimed(d); + if (d->bd_hbuf == NULL) { + printf("bpfread: lost race: bd_hbuf=%p bd_sbuf=%p bd_fbuf=%p\n", + d->bd_hbuf, d->bd_sbuf, d->bd_fbuf); + } else { + d->bd_fbuf = d->bd_hbuf; + d->bd_hbuf = NULL; + d->bd_hlen = 0; + bpf_buf_reclaimed(d); + } BPFD_UNLOCK(d); return (error); This still seems vulnerable to me -- a ROTATE_BUFFERS() or reset_d() could be done between the BPFD_UNLOCK() and the bpf_uiomove(). Would a new lock to protect the buffers be necessary, or a flag+condition variable to indicate "hold buffer in use"? Guy _______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"