Skip to content

Unsoundness due to construction of &mut [u8] and Box<[u8]> in Clone impl #522

Closed
@saethlin

Description

@saethlin

This example is minimized from the protobuf crate's test suite, using bytes v1.1.0:

fn main() {
    let bytes = bytes::Bytes::from(vec![1, 2, 3, 4]);
    let buf: &[u8] = &bytes; // Create a shared reference to the byte data
    bytes.clone(); // Create a unique reference to the byte data (oops?)
    drop(buf); // Use the original shared reference to the byte data
}

I think this is pretty solidly unsound under any reasonable aliasing formulation. Miri's stacked borrow checker flags the use of buf as the problematic line because its access was invalidated by constructing the &mut [u8], but I think a different sort of aliasing model could flag the creation of the &mut itself as the problem, because it aliases.

A diff which could fix this looks like this:

diff --git a/src/bytes.rs b/src/bytes.rs
index c4fbd4a..1a9a9b7 100644
--- a/src/bytes.rs
+++ b/src/bytes.rs
@@ -935,7 +935,7 @@ unsafe fn promotable_odd_drop(data: &mut AtomicPtr<()>, ptr: *const u8, len: usi
 
 unsafe fn rebuild_boxed_slice(buf: *mut u8, offset: *const u8, len: usize) -> Box<[u8]> {
     let cap = (offset as usize - buf as usize) + len;
-    Box::from_raw(slice::from_raw_parts_mut(buf, cap))
+    Box::from_raw(slice::from_raw_parts(buf, cap) as *const [u8] as *mut [u8])
 }
 
 // ===== impl SharedVtable =====

But per Stacked Borrows, this just gets over one hurdle only to crash straight into another, since Box has unique access just the same as &mut. I suspect this could be patched up as well by forsaking Box entirely within the internals of this library. But that might be kind of invasive, so I would prefer to get some feedback from maintainers first. What do you all think?

Updated to reflect the fact that Box asserts uniqueness, even in the absence of Stacked Borrows.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions