ringbuf icon indicating copy to clipboard operation
ringbuf copied to clipboard

Access to unitialized data in `read_from`

Open mpdn opened this issue 2 years ago • 1 comments

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:

https://github.com/agerasev/ringbuf/blob/447a15630b29c807ee137c7366a0bfca768bd5c1/src/traits/producer.rs#L122-L149

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

mpdn avatar Jan 16 '24 20:01 mpdn

Hi! Thank you for finding and reporting such a subtle issue!

I've made a fast fix by initializing destination memory before calling read. But this could result in a huge overhead when, for example, reader reads just a single byte, but the whole buffer needs to be initialized every time.

Waiting for Read::read_buf stabilization :)

agerasev avatar Jan 17 '24 11:01 agerasev