From 48d8cc96a983d2a38065a4c39d35795071d8a6f8 Mon Sep 17 00:00:00 2001 From: tooson Date: Thu, 27 Aug 2026 12:34:22 +0900 Subject: [PATCH] Fix uninitialized read when the filler closure panics AllocRingBuffer::fill_with and ConstGenericRingBuffer::fill_with set the write pointer to the full capacity before running the filler. If the closure panics partway through, the buffer still reports every slot as live while the ones the loop had not reached hold uninitialized memory. A later read - or the buffer's own Drop - then ptr::reads one of them. Commit each slot as it is written instead. --- src/lib.rs | 44 ++++++++++++++++++++++++++++++ src/with_alloc/alloc_ringbuffer.rs | 6 +++- src/with_const_generics.rs | 8 ++++-- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 23cafe7..a276e89 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2059,4 +2059,48 @@ mod tests { rb }); } + + #[test] + fn run_test_fill_with_panicking_closure() { + //! A panic in the filler must not leave the buffer claiming slots that + //! were never written. + + use core::cell::Cell; + use std::panic::{catch_unwind, AssertUnwindSafe}; + + const capacity: usize = 8; + + fn test_fill_with(mut b: impl RingBuffer>) { + let calls = Cell::new(0); + + let r = catch_unwind(AssertUnwindSafe(|| { + b.fill_with(|| { + let n = calls.get(); + calls.set(n + 1); + if n == 3 { + panic!("filler panics"); + } + vec![0xAA; 20] + }); + })); + assert!(r.is_err(), "the filler should have panicked"); + + // Only the slots the filler actually reached are initialised, so the + // buffer must not report more than that. + assert!( + b.len() <= 3, + "buffer reports {} live slots but only 3 were written", + b.len() + ); + + // Reading and dropping must not touch an uninitialised slot. + while let Some(v) = b.dequeue() { + assert_eq!(v.len(), 20); + } + } + + test_fill_with(AllocRingBuffer::new(capacity)); + test_fill_with(ConstGenericRingBuffer::, capacity>::new()); + test_fill_with(GrowableAllocRingBuffer::with_capacity(capacity)); + } } diff --git a/src/with_alloc/alloc_ringbuffer.rs b/src/with_alloc/alloc_ringbuffer.rs index 451db4e..aa60ad2 100644 --- a/src/with_alloc/alloc_ringbuffer.rs +++ b/src/with_alloc/alloc_ringbuffer.rs @@ -288,10 +288,14 @@ unsafe impl RingBuffer for AllocRingBuffer { self.clear(); self.readptr = 0; - self.writeptr = self.capacity; + self.writeptr = 0; for i in 0..self.capacity { unsafe { ptr::write(get_unchecked_mut(self, i), f()) }; + // Commit each slot as it is written. If `f` panics, `writeptr` then + // covers only the slots that were actually initialised, instead of + // claiming the whole buffer is live. + self.writeptr = i + 1; } } } diff --git a/src/with_const_generics.rs b/src/with_const_generics.rs index 1ab5240..c98779b 100644 --- a/src/with_const_generics.rs +++ b/src/with_const_generics.rs @@ -328,8 +328,12 @@ unsafe impl RingBuffer for ConstGenericRingBuffer T>(&mut self, mut f: F) { self.clear(); self.readptr = 0; - self.writeptr = CAP; - self.buf.fill_with(|| MaybeUninit::new(f())); + self.writeptr = 0; + + for i in 0..CAP { + self.buf[i] = MaybeUninit::new(f()); + self.writeptr = i + 1; + } } }