From 86a6fcd4ffb8373e942c763cbedbe2e80cb01ffa Mon Sep 17 00:00:00 2001 From: Ryan Stone Date: Thu, 1 Dec 2016 21:08:42 +0000 Subject: [PATCH] Fix a false positive in a buf_ring assert 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(). Sponsored by: Dell EMC Isilon MFC After: 1 week Differential Revision: https://reviews.freebsd.org/D8685 Reviewed by: hselasky --- sys/sys/buf_ring.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/sys/sys/buf_ring.h b/sys/sys/buf_ring.h index 234e318ecf1..fae6a082ab4 100644 --- a/sys/sys/buf_ring.h +++ b/sys/sys/buf_ring.h @@ -67,11 +67,13 @@ buf_ring_enqueue(struct buf_ring *br, void *buf) uint32_t prod_head, prod_next, cons_tail; #ifdef DEBUG_BUFRING int i; - for (i = br->br_cons_head; i != br->br_prod_head; - i = ((i + 1) & br->br_cons_mask)) - if(br->br_ring[i] == buf) - panic("buf=%p already enqueue at %d prod=%d cons=%d", - buf, i, br->br_prod_tail, br->br_cons_tail); + if (br->br_cons_head != br->br_prod_head) { + for (i = (br->br_cons_head + 1) & br->br_cons_mask; i != br->br_prod_head; + i = ((i + 1) & br->br_cons_mask)) + if(br->br_ring[i] == buf) + panic("buf=%p already enqueue at %d prod=%d cons=%d", + buf, i, br->br_prod_tail, br->br_cons_tail); + } #endif critical_enter(); do {