Safe API lets you alias mutable memory
japaric opened this issue · 3 comments
use core::mem;
// version = "=0.2.1"
use bbqueue::BBQueue;
fn main() {
// `&'static mut` references are safe to generate using `#[entry]` or `singleton!`
static mut A: [u8; 32] = [0; 32];
static mut B: [u8; 32] = [0; 32];
unsafe { safe_but_unsound(&mut A, &mut B) }
}
fn safe_but_unsound(a: &'static mut [u8], mut b: &'static mut [u8]) {
let mut a = BBQueue::new(a);
let mut x = a.grant(16).unwrap();
// this lets us effectively construct a fake `GrantW`
mem::swap(&mut x.buf, &mut b);
// we commit the fake grant; `b` still lets us access the granted buffer
a.commit(16, x);
// this aliases memory
let mut alias = a.grant(16).unwrap();
// this prints the same pointer and length
println!("{:?} - {}", b.as_mut_ptr(), b.len());
println!("{:?} - {}", alias.buf.as_mut_ptr(), alias.buf.len());
}
I think this can be patched by making the GrantW.buf
field private.
Another variant:
use core::mem;
// version = "=0.2.1"
use bbqueue::BBQueue;
fn main() {
// `&'static mut` references are safe to generate using `#[entry]` or `singleton!`
static mut A: [u8; 32] = [0; 32];
static mut B: [u8; 32] = [0; 32];
unsafe { safe_but_unsound(&mut A, &mut B) }
}
fn safe_but_unsound(a: &'static mut [u8], mut b: &'static mut [u8]) {
let mut a = BBQueue::new(a);
let mut b = BBQueue::new(b);
let mut x = a.grant(16).unwrap();
let mut y = b.grant(16).unwrap();
// here's the trick: we commit b's grant instead a's grant
a.commit(0, y);
// this aliases memory
let mut alias = a.grant(16).unwrap();
// this prints the same pointer and length
println!("{:?} - {}", x.buf.as_mut_ptr(), x.buf.len());
println!("{:?} - {}", alias.buf.as_mut_ptr(), alias.buf.len());
}
This one can be avoided by adding a runtime check to commit
. The check would consist of verifying that Grant.buf
is within the bounds of BBQueue.buf
.
A zero cost fix would be turning each BBQueue into a singleton then you would not be able to commit another's BBQueue grant because the types wouldn't match. You would get a compile error in this case. alloc-singleton
uses this trick.
Another variant:
// version = "=0.2.1"
use bbqueue::BBQueue;
fn main() {
// `&'static mut` references are safe to generate using `#[entry]` or `singleton!`
static mut A: [u8; 32] = [0; 32];
unsafe { safe_but_unsound(&mut A) }
}
fn safe_but_unsound(a: &'static mut [u8]) {
let mut a = BBQueue::new(a);
// dummy commit so we can use `read`
let x = a.grant(17).unwrap();
a.commit(17, x);
let y = a.read().unwrap();
// this makes a copy of the `&'static [u8]`
let z = y.buf;
a.release(17, y);
// this aliases memory; `&'static mut [u8]` that overlaps with `z`
let alias = a.grant(16).unwrap();
// these two slices overlap
println!("{:?} - {}", z.as_ptr(), z.len());
println!("{:?} - {}", alias.buf.as_mut_ptr(), alias.buf.len());
}
This can be fixed by making the GrantR.buf
field private. Instead GrantR
and GrantW
should expose the underlying buffer via Deref
/ DerefMut
that way the view into the buffer can't outlive the grant.
Interesting! I was wondering if providing raw access to the buffers was an issue - I didn't think about swapping the slices!
I'll look in to implementing Deref
and DerefMut
- my one concern is whether there will be issues with trying to give ownership of the Grant
back while a reference is still active.