mgeier / rtrb

A realtime-safe single-producer single-consumer (SPSC) ring buffer
Apache License 2.0
202 stars 14 forks source link

Exposing the `buffer` field of Consumer and Producer is unsound #26

Closed caelunshun closed 3 years ago

caelunshun commented 3 years ago

The Arc<RingBuffer> field on Consumer and Producer is made public, which is unsound. Overwriting the buffer field without updating head and tail can cause a read from uninitialized memory without unsafe code in downstream crates.

If the RingBuffer needs to be exposed, then I would recommend a method returning a shared reference instead of a public field.

mgeier commented 3 years ago

Thanks for the report!

I was very unsure when making this field public, but I thought it's fine because the Arc only ever gives read-only access. However, I didn't realize that the whole field can still be re-assigned!

If the RingBuffer needs to be exposed,

I'm actually not completely sure yet. For now, a reference to the ring buffer is only needed to get the capacity(), which is a method of the RingBuffer, and not of Producer/Consumer.

However, in the future there might be more methods on RingBuffer, for example mlock()/munlock() (see #3).

I wanted to avoid having to duplicate all methods from RingBuffer on both Producer and Consumer ...

Any suggestions?

then I would recommend a method returning a shared reference instead of a public field.

I've just created #27 as a possible solution.