Skip to content

Commit 14d48af

Browse files
committed
fix: pmem: add validation for head and status descriptors lengths
Head and status descriptors must be 4 bytes long by the spec. Add validation for this. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
1 parent 7d2faae commit 14d48af

File tree

1 file changed

+51
-0
lines changed

1 file changed

+51
-0
lines changed

src/vmm/src/devices/virtio/pmem/device.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ pub enum PmemError {
4444
ReadOnlyDescriptor,
4545
/// Unexpected write-only descriptor
4646
WriteOnlyDescriptor,
47+
/// Head descriptor has invalid length of {0} instead of 4
48+
Non4byteHeadDescriptor(u32),
49+
/// Status descriptor has invalid length of {0} instead of 4
50+
Non4byteStatusDescriptor(u32),
4751
/// UnknownRequestType: {0}
4852
UnknownRequestType(u32),
4953
/// Descriptor chain too short
@@ -283,19 +287,31 @@ impl Pmem {
283287
// This is safe since we checked in the event handler that the device is activated.
284288
let active_state = self.device_state.active_state().unwrap();
285289

290+
// Virtio spec, section 5.19.6 Driver Operations
291+
// https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01.html#x1-6970006
286292
if head.is_write_only() {
287293
return Err(PmemError::WriteOnlyDescriptor);
288294
}
295+
if head.len != 4 {
296+
return Err(PmemError::Non4byteHeadDescriptor(head.len));
297+
}
289298
let request: u32 = active_state.mem.read_obj(head.addr)?;
290299
if request != VIRTIO_PMEM_REQ_TYPE_FLUSH {
291300
return Err(PmemError::UnknownRequestType(request));
292301
}
302+
303+
// Virtio spec, section 5.19.7 Device Operations
304+
// https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01.html#x1-6980007
293305
let Some(status_descriptor) = head.next_descriptor() else {
294306
return Err(PmemError::DescriptorChainTooShort);
295307
};
296308
if !status_descriptor.is_write_only() {
297309
return Err(PmemError::ReadOnlyDescriptor);
298310
}
311+
if status_descriptor.len != 4 {
312+
return Err(PmemError::Non4byteStatusDescriptor(status_descriptor.len));
313+
}
314+
299315
let mut result = SUCCESS;
300316
// SAFETY: We are calling the system call with valid arguments and checking the returned
301317
// value
@@ -556,5 +572,40 @@ mod tests {
556572
PmemError::ReadOnlyDescriptor,
557573
));
558574
}
575+
576+
// Invalid length head descriptor
577+
{
578+
vq.avail.ring[0].set(0);
579+
vq.dtable[0].set(0x1000, 0x69, VIRTQ_DESC_F_NEXT, 1);
580+
mem.write_obj::<u32>(0, GuestAddress(0x1000)).unwrap();
581+
582+
pmem.queues[0] = vq.create_queue();
583+
vq.used.idx.set(0);
584+
vq.avail.idx.set(1);
585+
let head = pmem.queues[0].pop().unwrap().unwrap();
586+
assert!(matches!(
587+
pmem.process_chain(head).unwrap_err(),
588+
PmemError::Non4byteHeadDescriptor(0x69),
589+
));
590+
}
591+
592+
// Invalid length status descriptor
593+
{
594+
vq.avail.ring[0].set(0);
595+
vq.dtable[0].set(0x1000, 4, VIRTQ_DESC_F_NEXT, 1);
596+
vq.avail.ring[1].set(1);
597+
vq.dtable[1].set(0x2000, 0x69, VIRTQ_DESC_F_WRITE, 0);
598+
mem.write_obj::<u32>(0, GuestAddress(0x1000)).unwrap();
599+
mem.write_obj::<u32>(0x69, GuestAddress(0x2000)).unwrap();
600+
601+
pmem.queues[0] = vq.create_queue();
602+
vq.used.idx.set(0);
603+
vq.avail.idx.set(1);
604+
let head = pmem.queues[0].pop().unwrap().unwrap();
605+
assert!(matches!(
606+
pmem.process_chain(head).unwrap_err(),
607+
PmemError::Non4byteStatusDescriptor(0x69),
608+
));
609+
}
559610
}
560611
}

0 commit comments

Comments
 (0)