Page MenuHomeFreeBSD

Fix a false positive in a buf_ring assert
ClosedPublic

Authored by rstone on Nov 30 2016, 9:43 PM.
Tags
None
Referenced Files
F106130350: D8685.id22633.diff
Wed, Dec 25, 9:56 PM
F106102931: D8685.diff
Wed, Dec 25, 11:00 AM
Unknown Object (File)
Sun, Dec 15, 8:12 AM
Unknown Object (File)
Thu, Dec 5, 12:43 AM
Unknown Object (File)
Wed, Dec 4, 3:29 PM
Unknown Object (File)
Tue, Nov 26, 11:50 AM
Unknown Object (File)
Tue, Nov 26, 11:50 AM
Unknown Object (File)
Tue, Nov 26, 11:50 AM

Details

Summary

buf_ring contains an assert that checks whether an item being
enqueued already exists on the ring. There is a subtle bug in
this assert. An item can be returned by a peek() function and
freed, and then the consumer thread can be preempted before
calling advance(). If this happens the item appears to still be
on the queue, but another thread may allocate the item from the
free pool and wind up trying to enqueue it again, causing the
assert to trigger incorrectly.

Fix this by skipping the head of the consumer's portion of the
ring, as this index is what will be returned by peek().

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rstone retitled this revision from to Fix a false positive in a buf_ring assert.
rstone edited the test plan for this revision. (Show Details)
rstone updated this object.
hselasky added inline comments.
sys/sys/buf_ring.h
71 ↗(On Diff #22631)

should "br->br_cons_head + 1" be masked by br->br_cons_mask ??

Ensure we don't walk off the end of the ring

rstone added inline comments.
sys/sys/buf_ring.h
71 ↗(On Diff #22631)

Absolutely correct. Nice catch, thank you

This revision is now accepted and ready to land.Dec 1 2016, 8:23 AM
This revision was automatically updated to reflect the committed changes.
rstone marked an inline comment as done.