Skip to content

Commit bb0f847

Browse files
lauraltandreeaflorescu
authored andcommitted
mark VsockPacket::new as unsafe
The VsockPacket::new method is unsafe because it is relying on pointers that live for as long as the returned VsockPacket object. Marked the method accordingly and added a Safety section to it. Also simplified a bit the implementation, by always using the size of `data` array when creating the `data` VolatileSlice, and added a check for the size of the `header` slice. Signed-off-by: Laura Loghin <lauralg@amazon.com>
1 parent d8ef45f commit bb0f847

File tree

1 file changed

+26
-7
lines changed

1 file changed

+26
-7
lines changed

crates/devices/virtio-vsock/src/packet.rs

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -430,12 +430,22 @@ impl<'a, B: BitmapSlice> VsockPacket<'a, B> {
430430

431431
impl<'a> VsockPacket<'a, ()> {
432432
/// Create a packet based on one pointer for the header, and an optional one for data.
433-
pub fn new(hdr: &mut [u8], data: Option<(&mut [u8], usize)>) -> VsockPacket<'a, ()> {
434-
VsockPacket {
435-
header_slice: unsafe { VolatileSlice::new(hdr.as_mut_ptr(), PKT_HEADER_SIZE) },
436-
header: Default::default(),
437-
data_slice: data.map(|data| unsafe { VolatileSlice::new(data.0.as_mut_ptr(), data.1) }),
433+
///
434+
/// # Safety
435+
///
436+
/// To use this safely, the caller must guarantee that the memory pointed to by the `hdr` and
437+
/// `data` slices is available for the duration of the lifetime of the new `VolatileSlice`. The
438+
/// caller must also guarantee that all other users of the given chunk of memory are using
439+
/// volatile accesses.
440+
pub unsafe fn new(header: &mut [u8], data: Option<&mut [u8]>) -> Result<VsockPacket<'a, ()>> {
441+
if header.len() != PKT_HEADER_SIZE {
442+
return Err(Error::InvalidHeaderInputSize(header.len()));
438443
}
444+
Ok(VsockPacket {
445+
header_slice: VolatileSlice::new(header.as_mut_ptr(), PKT_HEADER_SIZE),
446+
header: Default::default(),
447+
data_slice: data.map(|data| VolatileSlice::new(data.as_mut_ptr(), data.len())),
448+
})
439449
}
440450
}
441451

@@ -988,17 +998,26 @@ mod tests {
988998
fn test_packet_new() {
989999
let mut pkt_raw = [0u8; PKT_HEADER_SIZE + LEN as usize];
9901000
let (hdr_raw, data_raw) = pkt_raw.split_at_mut(PKT_HEADER_SIZE);
991-
let packet = VsockPacket::new(hdr_raw, Some((data_raw, LEN as usize)));
1001+
// Safe because ``hdr_raw` and `data_raw` live for as long as the scope of the current test.
1002+
let packet = unsafe { VsockPacket::new(hdr_raw, Some(data_raw)).unwrap() };
9921003
assert_eq!(packet.header_slice.as_ptr(), hdr_raw.as_mut_ptr());
9931004
assert_eq!(packet.header_slice.len(), PKT_HEADER_SIZE);
9941005
assert_eq!(packet.header, PacketHeader::default());
9951006
assert_eq!(packet.data_slice.unwrap().as_ptr(), data_raw.as_mut_ptr());
9961007
assert_eq!(packet.data_slice.unwrap().len(), LEN as usize);
9971008

998-
let packet = VsockPacket::new(hdr_raw, None);
1009+
// Safe because ``hdr_raw` and `data_raw` live as long as the scope of the current test.
1010+
let packet = unsafe { VsockPacket::new(hdr_raw, None).unwrap() };
9991011
assert_eq!(packet.header_slice.as_ptr(), hdr_raw.as_mut_ptr());
10001012
assert_eq!(packet.header, PacketHeader::default());
10011013
assert!(packet.data_slice.is_none());
1014+
1015+
let mut hdr_raw = [0u8; PKT_HEADER_SIZE - 1];
1016+
// Safe because ``hdr_raw` lives for as long as the scope of the current test.
1017+
assert_eq!(
1018+
unsafe { VsockPacket::new(&mut hdr_raw, None).unwrap_err() },
1019+
Error::InvalidHeaderInputSize(PKT_HEADER_SIZE - 1)
1020+
);
10021021
}
10031022

10041023
#[test]

0 commit comments

Comments
 (0)