Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 11 Aug 2014 19:38:04 +0000
From:      bugzilla-noreply@freebsd.org
To:        freebsd-bugs@FreeBSD.org
Subject:   [Bug 192558] [patch] hard-to-hit crash in bpf catchpacket()
Message-ID:  <bug-192558-8-SGtrZbLeOB@https.bugs.freebsd.org/bugzilla/>
In-Reply-To: <bug-192558-8@https.bugs.freebsd.org/bugzilla/>
References:  <bug-192558-8@https.bugs.freebsd.org/bugzilla/>

next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192558

John Baldwin <jhb@FreeBSD.org> changed:

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

--- Comment #1 from John Baldwin <jhb@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.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-192558-8-SGtrZbLeOB>