Skip to content

Commit c0f1bae

Browse files
committed
block: revert off-by-one error in addr validation
Signed-off-by: Luminita Voicu <[email protected]>
1 parent 0afa0c9 commit c0f1bae

File tree

2 files changed

+5
-71
lines changed

2 files changed

+5
-71
lines changed

src/devices/src/virtio/block/device.rs

Lines changed: 3 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,9 @@ impl Block {
230230
e.status()
231231
}
232232
};
233-
234-
if let Err(e) = mem.write_obj(status, request.status_addr) {
235-
error!("Failed to write virtio block status: {:?}", e)
236-
}
233+
// We use unwrap because the request parsing process already checked that the
234+
// status_addr was valid.
235+
mem.write_obj(status, request.status_addr).unwrap();
237236
}
238237
Err(e) => {
239238
error!("Failed to parse available descriptor chain: {:?}", e);
@@ -585,39 +584,6 @@ pub(crate) mod tests {
585584
assert_eq!(vq.used.ring[0].get().len, 0);
586585
}
587586

588-
#[test]
589-
fn test_addr_out_of_bounds() {
590-
let mut block = default_block();
591-
// Default mem size is 0x10000
592-
let mem = default_mem();
593-
let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
594-
block.set_queue(0, vq.create_queue());
595-
block.activate(mem.clone()).unwrap();
596-
initialize_virtqueue(&vq);
597-
vq.dtable[1].set(0xff00, 0x1000, VIRTQ_DESC_F_NEXT | VIRTQ_DESC_F_WRITE, 2);
598-
599-
let request_type_addr = GuestAddress(vq.dtable[0].addr.get());
600-
601-
// Mark the next available descriptor.
602-
vq.avail.idx.set(1);
603-
// Read.
604-
{
605-
vq.used.idx.set(0);
606-
607-
mem.write_obj::<u32>(VIRTIO_BLK_T_IN, request_type_addr)
608-
.unwrap();
609-
vq.dtable[1]
610-
.flags
611-
.set(VIRTQ_DESC_F_NEXT | VIRTQ_DESC_F_WRITE);
612-
613-
check_metric_after_block!(
614-
&METRICS.block.execute_fails,
615-
1,
616-
invoke_handler_for_queue_event(&mut block)
617-
);
618-
}
619-
}
620-
621587
#[test]
622588
fn test_request_execute_failures() {
623589
let mut block = default_block();
@@ -705,38 +671,6 @@ pub(crate) mod tests {
705671
VIRTIO_BLK_S_UNSUPP
706672
);
707673
}
708-
#[test]
709-
fn test_end_of_region() {
710-
let mut block = default_block();
711-
let mem = default_mem();
712-
let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
713-
block.set_queue(0, vq.create_queue());
714-
block.activate(mem.clone()).unwrap();
715-
initialize_virtqueue(&vq);
716-
vq.dtable[1].set(0xf000, 0x1000, VIRTQ_DESC_F_NEXT | VIRTQ_DESC_F_WRITE, 2);
717-
718-
let request_type_addr = GuestAddress(vq.dtable[0].addr.get());
719-
let status_addr = GuestAddress(vq.dtable[2].addr.get());
720-
721-
vq.used.idx.set(0);
722-
723-
mem.write_obj::<u32>(VIRTIO_BLK_T_IN, request_type_addr)
724-
.unwrap();
725-
vq.dtable[1]
726-
.flags
727-
.set(VIRTQ_DESC_F_NEXT | VIRTQ_DESC_F_WRITE);
728-
729-
check_metric_after_block!(
730-
&METRICS.block.read_count,
731-
1,
732-
invoke_handler_for_queue_event(&mut block)
733-
);
734-
735-
assert_eq!(vq.used.idx.get(), 1);
736-
assert_eq!(vq.used.ring[0].get().id, 0);
737-
assert_eq!(vq.used.ring[0].get().len, vq.dtable[1].len.get());
738-
assert_eq!(mem.read_obj::<u32>(status_addr).unwrap(), VIRTIO_BLK_S_OK);
739-
}
740674

741675
#[test]
742676
fn test_read_write() {

src/devices/src/virtio/block/request.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ impl Request {
160160

161161
// Check that the address of the data descriptor is valid in guest memory.
162162
let _ = mem
163-
.checked_offset(data_desc.addr, (data_desc.len - 1) as usize)
163+
.checked_offset(data_desc.addr, data_desc.len as usize)
164164
.ok_or(Error::GuestMemory(GuestMemoryError::InvalidGuestAddress(
165165
data_desc.addr,
166166
)))?;
@@ -181,7 +181,7 @@ impl Request {
181181
// Check that the address of the status descriptor is valid in guest memory.
182182
// We will write an u32 status here after executing the request.
183183
let _ = mem
184-
.checked_offset(status_desc.addr, (mem::size_of::<u32>() - 1) as usize)
184+
.checked_offset(status_desc.addr, mem::size_of::<u32>())
185185
.ok_or(Error::GuestMemory(GuestMemoryError::InvalidGuestAddress(
186186
status_desc.addr,
187187
)))?;

0 commit comments

Comments
 (0)