[Bug 192558] [patch] hard-to-hit crash in bpf catchpacket()

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192558

John Baldwin <jhb at FreeBSD.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Needs Triage                |In Discussion
                 CC|                            |jhb at FreeBSD.org,
                   |                            |rwatson at FreeBSD.org

--- Comment #1 from John Baldwin <jhb at FreeBSD.org> ---
I think your diagnosis is correct, but I'm unsure of the fix.  In particular, I
suspect we may want to avoid sleeping if the fbuf is full so that this "fails
fast" to avoid blocking network traffic.  Also, when you are awakened, other
threads might have already called catchpacket() (e.g. a multiq NIC), so you
don't actually know that you should be in the current state.  I worry that you
need to jump back to the preceding if, so something like:

    again:
        if (curlen + totlen > d->bd_bufsize || !bpf_canwritebuf(d)) {
            if (d->bd_fbuf == NULL) {
                ...
            }
            if (d->hd_buf_in_use) {
                mtx_sleep(...);
                goto again;
            }
            ...
        }


Arguably the earlier call to mtx_sleep() earlier in this function is also racy
as you don't know that the condition is still true when you awake given a
concurrent call to catchpacket() from another RX queue on the same NIC.  That
is, I think that loop should also be altered, though it can probably avoid a
goto:

        while (d->bd_fbuf == NULL && bpf_canfreebuf(d)) {
            if (d->bd_hbuf_in_use) {
                mtx_sleep(...);
                continue;
            }
            ...
        }

Possibly, the 'again' label needs to move all the way up to before this 'while'
loop, and if so, the hrdlen/totlen/curlen assigments should be moved up before
the first while.

-- 
You are receiving this mail because:
You are the assignee for the bug.