Skip to content

Commit 0090567

Browse files
danielverkampcrosvm LUCI
authored and
crosvm LUCI
committed
vhost: improve set_vring_addr() validation
Use the GuestMemory get_slice_at_addr() function to ensure the memory regions corresponding to the descriptor table and used/avail rings are actually contiguous, rather than just validating the ending address of each region exists in guest memory. The log region is currently not validated, which matches previous behavior. BUG=None TEST=tools/dev_container tools/presubmit Change-Id: I1e80326dfae085380fbbf4dd1c7960dd72485793 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5922041 Commit-Queue: Daniel Verkamp <[email protected]> Reviewed-by: Keiichi Watanabe <[email protected]>
1 parent 1cba602 commit 0090567

File tree

1 file changed

+18
-55
lines changed

1 file changed

+18
-55
lines changed

vhost/src/lib.rs

+18-55
Original file line numberDiff line numberDiff line change
@@ -202,42 +202,6 @@ pub trait Vhost: AsRawDescriptor + std::marker::Sized {
202202
Ok(())
203203
}
204204

205-
// TODO(smbarber): This is copypasta. Eliminate the copypasta.
206-
#[allow(clippy::if_same_then_else)]
207-
fn is_valid(
208-
&self,
209-
mem: &GuestMemory,
210-
queue_max_size: u16,
211-
queue_size: u16,
212-
desc_addr: GuestAddress,
213-
avail_addr: GuestAddress,
214-
used_addr: GuestAddress,
215-
) -> bool {
216-
let desc_table_size = 16 * queue_size as usize;
217-
let avail_ring_size = 6 + 2 * queue_size as usize;
218-
let used_ring_size = 6 + 8 * queue_size as usize;
219-
if queue_size > queue_max_size || queue_size == 0 || (queue_size & (queue_size - 1)) != 0 {
220-
false
221-
} else if desc_addr
222-
.checked_add(desc_table_size as u64)
223-
.map_or(true, |v| !mem.address_in_range(v))
224-
{
225-
false
226-
} else if avail_addr
227-
.checked_add(avail_ring_size as u64)
228-
.map_or(true, |v| !mem.address_in_range(v))
229-
{
230-
false
231-
} else if used_addr
232-
.checked_add(used_ring_size as u64)
233-
.map_or(true, |v| !mem.address_in_range(v))
234-
{
235-
false
236-
} else {
237-
true
238-
}
239-
}
240-
241205
/// Set the addresses for a given vring.
242206
///
243207
/// # Arguments
@@ -261,28 +225,27 @@ pub trait Vhost: AsRawDescriptor + std::marker::Sized {
261225
avail_addr: GuestAddress,
262226
log_addr: Option<GuestAddress>,
263227
) -> Result<()> {
264-
// TODO(smbarber): Refactor out virtio from crosvm so we can
265-
// validate a Queue struct directly.
266-
if !self.is_valid(
267-
mem,
268-
queue_max_size,
269-
queue_size,
270-
desc_addr,
271-
used_addr,
272-
avail_addr,
273-
) {
228+
if queue_size > queue_max_size || queue_size == 0 || !queue_size.is_power_of_two() {
274229
return Err(Error::InvalidQueue);
275230
}
276231

277-
let desc_addr = mem
278-
.get_host_address(desc_addr)
232+
let queue_size = usize::from(queue_size);
233+
234+
let desc_table_size = 16 * queue_size;
235+
let desc_table = mem
236+
.get_slice_at_addr(desc_addr, desc_table_size)
279237
.map_err(Error::DescriptorTableAddress)?;
280-
let used_addr = mem
281-
.get_host_address(used_addr)
238+
239+
let used_ring_size = 6 + 8 * queue_size;
240+
let used_ring = mem
241+
.get_slice_at_addr(used_addr, used_ring_size)
282242
.map_err(Error::UsedAddress)?;
283-
let avail_addr = mem
284-
.get_host_address(avail_addr)
243+
244+
let avail_ring_size = 6 + 2 * queue_size;
245+
let avail_ring = mem
246+
.get_slice_at_addr(avail_addr, avail_ring_size)
285247
.map_err(Error::AvailAddress)?;
248+
286249
let log_addr = match log_addr {
287250
None => null(),
288251
Some(a) => mem.get_host_address(a).map_err(Error::LogAddress)?,
@@ -291,9 +254,9 @@ pub trait Vhost: AsRawDescriptor + std::marker::Sized {
291254
let vring_addr = virtio_sys::vhost::vhost_vring_addr {
292255
index: queue_index as u32,
293256
flags,
294-
desc_user_addr: desc_addr as u64,
295-
used_user_addr: used_addr as u64,
296-
avail_user_addr: avail_addr as u64,
257+
desc_user_addr: desc_table.as_ptr() as u64,
258+
used_user_addr: used_ring.as_ptr() as u64,
259+
avail_user_addr: avail_ring.as_ptr() as u64,
297260
log_guest_addr: log_addr as u64,
298261
};
299262

0 commit comments

Comments
 (0)