Skip to content

Commit f682a5c

Browse files
lauraltandreeaflorescu
authored andcommitted
vsock: drop packets with data len > a max size
Updated VsockPacket::from_tx_virtq_chain to also have as parameter a max value for the payload size. It is currently under discussion to add a max size field in the config space for the packet max size, and its value will be negotiated between the driver and the device. That way the driver is forced to not send bigger buffers. Even if the proposal won't actually be added to virtio-spec, it is still a good practice to check against a max value in the device code. Related virtio-spec thread: https://lists.oasis-open.org/archives/virtio-comment/202202/msg00053.html Signed-off-by: Laura Loghin <lauralg@amazon.com>
1 parent b155d93 commit f682a5c

File tree

1 file changed

+51
-13
lines changed

1 file changed

+51
-13
lines changed

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

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ pub enum Error {
5050
DescriptorLengthTooSmall,
5151
/// The slice for creating a header has an invalid length.
5252
InvalidHeaderInputSize(usize),
53+
/// The `len` header field value exceeds the maximum allowed data size.
54+
InvalidHeaderLen(u32),
5355
/// Invalid guest memory access.
5456
InvalidMemoryAccess(GuestMemoryError),
5557
/// Invalid volatile memory access.
@@ -75,6 +77,9 @@ impl Display for Error {
7577
Error::InvalidHeaderInputSize(size) => {
7678
write!(f, "Invalid header input size: {}", size)
7779
}
80+
Error::InvalidHeaderLen(size) => {
81+
write!(f, "Invalid header `len` field value: {}", size)
82+
}
7883
Error::InvalidMemoryAccess(error) => {
7984
write!(f, "Invalid guest memory access: {}", error)
8085
}
@@ -319,6 +324,7 @@ impl<'a, B: BitmapSlice> VsockPacket<'a, B> {
319324
pub fn from_tx_virtq_chain<M, T>(
320325
mem: &'a M,
321326
desc_chain: &mut DescriptorChain<T>,
327+
max_data_size: u32,
322328
) -> Result<Self>
323329
where
324330
M: GuestMemory,
@@ -356,6 +362,11 @@ impl<'a, B: BitmapSlice> VsockPacket<'a, B> {
356362
return Ok(pkt);
357363
}
358364

365+
// Reject packets that exceed the maximum allowed value for payload.
366+
if pkt.len() > max_data_size {
367+
return Err(Error::InvalidHeaderLen(pkt.len()));
368+
}
369+
359370
let data_desc = desc_chain.next().ok_or(Error::DescriptorChainTooShort)?;
360371

361372
if data_desc.is_write_only() {
@@ -468,6 +479,7 @@ mod tests {
468479
(InvalidHeaderInputSize(size), InvalidHeaderInputSize(other_size)) => {
469480
size == other_size
470481
}
482+
(InvalidHeaderLen(size), InvalidHeaderLen(other_size)) => size == other_size,
471483
(InvalidMemoryAccess(ref e), InvalidMemoryAccess(ref other_e)) => {
472484
format!("{}", e).eq(&format!("{}", other_e))
473485
}
@@ -494,6 +506,8 @@ mod tests {
494506
const BUF_ALLOC: u32 = 256;
495507
const FWD_CNT: u32 = 9;
496508

509+
const MAX_PKT_BUF_SIZE: u32 = 64 * 1024;
510+
497511
#[test]
498512
fn test_from_rx_virtq_chain() {
499513
let mem: GuestMemoryMmap =
@@ -650,7 +664,7 @@ mod tests {
650664
let queue = MockSplitQueue::new(&mem, 16);
651665
let mut chain = queue.build_desc_chain(&v[..2]);
652666
assert_eq!(
653-
VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap_err(),
667+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
654668
Error::UnexpectedWriteOnlyDescriptor
655669
);
656670

@@ -661,7 +675,7 @@ mod tests {
661675
];
662676
let mut chain = queue.build_desc_chain(&v[..2]);
663677
assert_eq!(
664-
VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap_err(),
678+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
665679
Error::DescriptorLengthTooSmall
666680
);
667681

@@ -683,7 +697,7 @@ mod tests {
683697
};
684698
mem.write_obj(header, GuestAddress(0x10_0000)).unwrap();
685699

686-
let packet = VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap();
700+
let packet = VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap();
687701
assert_eq!(packet.header, header);
688702
let header_slice = packet.header_slice();
689703
assert_eq!(
@@ -704,7 +718,7 @@ mod tests {
704718
let queue = MockSplitQueue::new(&mem, 16);
705719
let mut chain = queue.build_desc_chain(&v[..2]);
706720
assert_eq!(
707-
VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap_err(),
721+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
708722
Error::InvalidMemoryAccess(GuestMemoryError::InvalidBackendAddress)
709723
);
710724

@@ -715,14 +729,38 @@ mod tests {
715729
];
716730
let mut chain = queue.build_desc_chain(&v[..2]);
717731
assert_eq!(
718-
VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap_err(),
732+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
719733
Error::InvalidMemoryAccess(GuestMemoryError::InvalidGuestAddress(GuestAddress(
720734
0x20_0000
721735
)))
722736
);
723737

724738
// Write some non-zero value to the `len` field of the header, which means there is also
725-
// a data descriptor in the chain.
739+
// a data descriptor in the chain, first with a value that exceeds the maximum allowed one.
740+
let header = PacketHeader {
741+
src_cid: SRC_CID.into(),
742+
dst_cid: DST_CID.into(),
743+
src_port: SRC_PORT.into(),
744+
dst_port: DST_PORT.into(),
745+
len: (MAX_PKT_BUF_SIZE + 1).into(),
746+
type_: 0.into(),
747+
op: 0.into(),
748+
flags: 0.into(),
749+
buf_alloc: 0.into(),
750+
fwd_cnt: 0.into(),
751+
};
752+
mem.write_obj(header, GuestAddress(0x5_0000)).unwrap();
753+
let v = vec![
754+
Descriptor::new(0x5_0000, 0x100, 0, 0),
755+
Descriptor::new(0x8_0000, 0x100, 0, 0),
756+
];
757+
let mut chain = queue.build_desc_chain(&v[..2]);
758+
assert_eq!(
759+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
760+
Error::InvalidHeaderLen(MAX_PKT_BUF_SIZE + 1)
761+
);
762+
763+
// Write some non-zero, valid value to the `len` field of the header.
726764
let header = PacketHeader {
727765
src_cid: SRC_CID.into(),
728766
dst_cid: DST_CID.into(),
@@ -742,7 +780,7 @@ mod tests {
742780
];
743781
let mut chain = queue.build_desc_chain(&v[..1]);
744782
assert_eq!(
745-
VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap_err(),
783+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
746784
Error::DescriptorChainTooShort
747785
);
748786

@@ -753,7 +791,7 @@ mod tests {
753791
];
754792
let mut chain = queue.build_desc_chain(&v[..2]);
755793
assert_eq!(
756-
VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap_err(),
794+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
757795
Error::InvalidMemoryAccess(GuestMemoryError::InvalidBackendAddress)
758796
);
759797

@@ -764,7 +802,7 @@ mod tests {
764802
];
765803
let mut chain = queue.build_desc_chain(&v[..2]);
766804
assert_eq!(
767-
VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap_err(),
805+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
768806
Error::InvalidMemoryAccess(GuestMemoryError::InvalidGuestAddress(GuestAddress(
769807
0x20_0000
770808
)))
@@ -777,7 +815,7 @@ mod tests {
777815
];
778816
let mut chain = queue.build_desc_chain(&v[..2]);
779817
assert_eq!(
780-
VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap_err(),
818+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
781819
Error::UnexpectedWriteOnlyDescriptor
782820
);
783821

@@ -788,7 +826,7 @@ mod tests {
788826
];
789827
let mut chain = queue.build_desc_chain(&v[..2]);
790828
assert_eq!(
791-
VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap_err(),
829+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
792830
Error::DescriptorLengthTooSmall
793831
);
794832

@@ -799,7 +837,7 @@ mod tests {
799837
];
800838
let mut chain = queue.build_desc_chain(&v[..2]);
801839

802-
let packet = VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap();
840+
let packet = VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap();
803841
assert_eq!(packet.header, header);
804842
let header_slice = packet.header_slice();
805843
assert_eq!(
@@ -820,7 +858,7 @@ mod tests {
820858
// If we try to get a vsock packet again, it fails because we already consumed all the
821859
// descriptors from the chain.
822860
assert_eq!(
823-
VsockPacket::from_tx_virtq_chain(&mem, &mut chain).unwrap_err(),
861+
VsockPacket::from_tx_virtq_chain(&mem, &mut chain, MAX_PKT_BUF_SIZE).unwrap_err(),
824862
Error::DescriptorChainTooShort
825863
);
826864
}

0 commit comments

Comments
 (0)