Skip to content

Commit a563233

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 b5ac3a6 commit a563233

1 file changed

Lines changed: 49 additions & 0 deletions

File tree

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

Lines changed: 49 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,29 @@ 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
286291
if head.is_write_only() {
287292
return Err(PmemError::WriteOnlyDescriptor);
288293
}
294+
if head.len != 4 {
295+
return Err(PmemError::Non4byteHeadDescriptor(head.len));
296+
}
289297
let request: u32 = active_state.mem.read_obj(head.addr)?;
290298
if request != VIRTIO_PMEM_REQ_TYPE_FLUSH {
291299
return Err(PmemError::UnknownRequestType(request));
292300
}
301+
302+
// Virtio spec, section 5.19.7.2 Device Operations
293303
let Some(status_descriptor) = head.next_descriptor() else {
294304
return Err(PmemError::DescriptorChainTooShort);
295305
};
296306
if !status_descriptor.is_write_only() {
297307
return Err(PmemError::ReadOnlyDescriptor);
298308
}
309+
if status_descriptor.len != 4 {
310+
return Err(PmemError::Non4byteStatusDescriptor(status_descriptor.len));
311+
}
312+
299313
let mut result = SUCCESS;
300314
// SAFETY: We are calling the system call with valid arguments and checking the returned
301315
// value
@@ -556,5 +570,40 @@ mod tests {
556570
PmemError::ReadOnlyDescriptor,
557571
));
558572
}
573+
574+
// Invalid length head descriptor
575+
{
576+
vq.avail.ring[0].set(0);
577+
vq.dtable[0].set(0x1000, 0x69, VIRTQ_DESC_F_NEXT, 1);
578+
mem.write_obj::<u32>(0, GuestAddress(0x1000)).unwrap();
579+
580+
pmem.queues[0] = vq.create_queue();
581+
vq.used.idx.set(0);
582+
vq.avail.idx.set(1);
583+
let head = pmem.queues[0].pop().unwrap().unwrap();
584+
assert!(matches!(
585+
pmem.process_chain(head).unwrap_err(),
586+
PmemError::Non4byteHeadDescriptor(0x69),
587+
));
588+
}
589+
590+
// Invalid length status descriptor
591+
{
592+
vq.avail.ring[0].set(0);
593+
vq.dtable[0].set(0x1000, 4, VIRTQ_DESC_F_NEXT, 1);
594+
vq.avail.ring[1].set(1);
595+
vq.dtable[1].set(0x2000, 0x69, VIRTQ_DESC_F_WRITE, 0);
596+
mem.write_obj::<u32>(0, GuestAddress(0x1000)).unwrap();
597+
mem.write_obj::<u32>(0x69, GuestAddress(0x2000)).unwrap();
598+
599+
pmem.queues[0] = vq.create_queue();
600+
vq.used.idx.set(0);
601+
vq.avail.idx.set(1);
602+
let head = pmem.queues[0].pop().unwrap().unwrap();
603+
assert!(matches!(
604+
pmem.process_chain(head).unwrap_err(),
605+
PmemError::Non4byteStatusDescriptor(0x69),
606+
));
607+
}
559608
}
560609
}

0 commit comments

Comments
 (0)