Hi. Nice library, thanks for putting effort into making this.
I think I've discovered an issue allowing use of uninitialized data from safe code in the read_from function:
|
/// Reads at most `count` bytes from `Read` instance and appends them to the ring buffer. |
|
/// If `count` is `None` then as much as possible bytes will be read. |
|
/// |
|
/// Returns: |
|
/// + `None`: ring buffer is empty or `count` is `0`. In this case `read` isn't called at all. |
|
/// + `Some(Ok(n))`: `read` succeeded. `n` is number of bytes been read. `n == 0` means that `read` also returned `0`. |
|
/// + `Some(Err(e))` `read` is failed and `e` is original error. In this case it is guaranteed that no items was read from the reader. |
|
/// To achieve this we read only one contiguous slice at once. So this call may read less than `vacant_len` items in the buffer even if the reader is ready to provide more. |
|
fn read_from<S: Read>(&mut self, reader: &mut S, count: Option<usize>) -> Option<io::Result<usize>> |
|
where |
|
Self: Producer<Item = u8>, |
|
{ |
|
let (left, _) = self.vacant_slices_mut(); |
|
let count = cmp::min(count.unwrap_or(left.len()), left.len()); |
|
if count == 0 { |
|
return None; |
|
} |
|
let left_init = unsafe { slice_assume_init_mut(&mut left[..count]) }; |
|
|
|
let read_count = match reader.read(left_init) { |
|
Ok(n) => n, |
|
Err(e) => return Some(Err(e)), |
|
}; |
|
assert!(read_count <= count); |
|
unsafe { self.advance_write_index(read_count) }; |
|
Some(Ok(read_count)) |
|
} |
|
} |
The issue is that a (safe) implementation of Read can read the uninitialized data in the buffer. This test:
#[test]
fn test_read_touching_buf() {
struct Reader;
impl Read for Reader {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
assert!(buf[0] == 0);
Ok(0)
}
}
let (mut producer, _consumer) = HeapRb::<u8>::new(10).split();
producer.read_from(&mut Reader, None).unwrap();
}
Triggers this error when executing in Miri:
test foo ... error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
--> src/main.rs:73:21
|
73 | assert!(buf[0] == 0);
| ^^^^^^ using uninitialized data, but this operation requires initialized memory
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
Hi. Nice library, thanks for putting effort into making this.
I think I've discovered an issue allowing use of uninitialized data from safe code in the
read_fromfunction:ringbuf/src/traits/producer.rs
Lines 122 to 149 in 447a156
The issue is that a (safe) implementation of
Readcan read the uninitialized data in the buffer. This test:Triggers this error when executing in Miri: