mirror of
https://github.com/cloud-hypervisor/cloud-hypervisor.git
synced 2026-08-05 02:19:16 +00:00
virtio-devices: iommu: Tighten MAP request validation
The handler accepted unknown flag bits, unaligned ranges, and overlapping mappings, and returned the wrong status code when the target domain did not exist. The virtio spec requires explicit rejections for each of these. Signed-off-by: Rob Bradford <rbradford@meta.com> Assisted-by: Claude:claude-opus-4-7
This commit is contained in:
@@ -72,6 +72,7 @@ const VIRTIO_IOMMU_F_BYPASS_CONFIG: u32 = 6;
|
|||||||
|
|
||||||
// Support 2MiB and 4KiB page sizes.
|
// Support 2MiB and 4KiB page sizes.
|
||||||
const VIRTIO_IOMMU_PAGE_SIZE_MASK: u64 = (2 << 20) | (4 << 10);
|
const VIRTIO_IOMMU_PAGE_SIZE_MASK: u64 = (2 << 20) | (4 << 10);
|
||||||
|
const VIRTIO_IOMMU_PAGE_GRANULE: u64 = 1u64 << VIRTIO_IOMMU_PAGE_SIZE_MASK.trailing_zeros();
|
||||||
|
|
||||||
// ~64 MiB at ~64 bytes/entry, well above any legitimate workload.
|
// ~64 MiB at ~64 bytes/entry, well above any legitimate workload.
|
||||||
const MAX_MAPPINGS_PER_DOMAIN: usize = 1 << 20;
|
const MAX_MAPPINGS_PER_DOMAIN: usize = 1 << 20;
|
||||||
@@ -125,11 +126,8 @@ const VIRTIO_IOMMU_S_IOERR: u8 = 1;
|
|||||||
#[allow(unused)]
|
#[allow(unused)]
|
||||||
const VIRTIO_IOMMU_S_UNSUPP: u8 = 2;
|
const VIRTIO_IOMMU_S_UNSUPP: u8 = 2;
|
||||||
const VIRTIO_IOMMU_S_DEVERR: u8 = 3;
|
const VIRTIO_IOMMU_S_DEVERR: u8 = 3;
|
||||||
#[allow(unused)]
|
|
||||||
const VIRTIO_IOMMU_S_INVAL: u8 = 4;
|
const VIRTIO_IOMMU_S_INVAL: u8 = 4;
|
||||||
#[allow(unused)]
|
|
||||||
const VIRTIO_IOMMU_S_RANGE: u8 = 5;
|
const VIRTIO_IOMMU_S_RANGE: u8 = 5;
|
||||||
#[allow(unused)]
|
|
||||||
const VIRTIO_IOMMU_S_NOENT: u8 = 6;
|
const VIRTIO_IOMMU_S_NOENT: u8 = 6;
|
||||||
#[allow(unused)]
|
#[allow(unused)]
|
||||||
const VIRTIO_IOMMU_S_FAULT: u8 = 7;
|
const VIRTIO_IOMMU_S_FAULT: u8 = 7;
|
||||||
@@ -165,13 +163,9 @@ struct VirtioIommuReqDetach {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Virtio IOMMU request MAP flags
|
/// Virtio IOMMU request MAP flags
|
||||||
#[allow(unused)]
|
|
||||||
const VIRTIO_IOMMU_MAP_F_READ: u32 = 1;
|
const VIRTIO_IOMMU_MAP_F_READ: u32 = 1;
|
||||||
#[allow(unused)]
|
|
||||||
const VIRTIO_IOMMU_MAP_F_WRITE: u32 = 1 << 1;
|
const VIRTIO_IOMMU_MAP_F_WRITE: u32 = 1 << 1;
|
||||||
#[allow(unused)]
|
|
||||||
const VIRTIO_IOMMU_MAP_F_MMIO: u32 = 1 << 2;
|
const VIRTIO_IOMMU_MAP_F_MMIO: u32 = 1 << 2;
|
||||||
#[allow(unused)]
|
|
||||||
const VIRTIO_IOMMU_MAP_F_MASK: u32 =
|
const VIRTIO_IOMMU_MAP_F_MASK: u32 =
|
||||||
VIRTIO_IOMMU_MAP_F_READ | VIRTIO_IOMMU_MAP_F_WRITE | VIRTIO_IOMMU_MAP_F_MMIO;
|
VIRTIO_IOMMU_MAP_F_READ | VIRTIO_IOMMU_MAP_F_WRITE | VIRTIO_IOMMU_MAP_F_MMIO;
|
||||||
|
|
||||||
@@ -183,7 +177,7 @@ struct VirtioIommuReqMap {
|
|||||||
virt_start: u64,
|
virt_start: u64,
|
||||||
virt_end: u64,
|
virt_end: u64,
|
||||||
phys_start: u64,
|
phys_start: u64,
|
||||||
_flags: u32,
|
flags: u32,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// UNMAP request
|
/// UNMAP request
|
||||||
@@ -491,6 +485,11 @@ impl Request {
|
|||||||
.map_err(Error::GuestMemory)?;
|
.map_err(Error::GuestMemory)?;
|
||||||
debug!("Map request 0x{req:x?}");
|
debug!("Map request 0x{req:x?}");
|
||||||
|
|
||||||
|
if (req.flags & !VIRTIO_IOMMU_MAP_F_MASK) != 0 {
|
||||||
|
status = VIRTIO_IOMMU_S_INVAL;
|
||||||
|
return Err(Error::InvalidMapRequest);
|
||||||
|
}
|
||||||
|
|
||||||
// Copy the value to use it as a proper reference.
|
// Copy the value to use it as a proper reference.
|
||||||
let domain_id = req.domain;
|
let domain_id = req.domain;
|
||||||
|
|
||||||
@@ -500,7 +499,7 @@ impl Request {
|
|||||||
return Err(Error::InvalidMapRequestBypassDomain);
|
return Err(Error::InvalidMapRequestBypassDomain);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
status = VIRTIO_IOMMU_S_INVAL;
|
status = VIRTIO_IOMMU_S_NOENT;
|
||||||
return Err(Error::InvalidMapRequestMissingDomain);
|
return Err(Error::InvalidMapRequestMissingDomain);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -528,6 +527,27 @@ impl Request {
|
|||||||
return Err(Error::InvalidMapRequest);
|
return Err(Error::InvalidMapRequest);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let mask = VIRTIO_IOMMU_PAGE_GRANULE - 1;
|
||||||
|
if (req.virt_start & mask) != 0
|
||||||
|
|| (req.phys_start & mask) != 0
|
||||||
|
|| (size & mask) != 0
|
||||||
|
{
|
||||||
|
status = VIRTIO_IOMMU_S_RANGE;
|
||||||
|
return Err(Error::InvalidMapRequest);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Going forward MAP rejects overlap, so within a domain
|
||||||
|
// mappings are disjoint and the rightmost mapping with
|
||||||
|
// start <= virt_end is the only candidate to overlap.
|
||||||
|
if let Some(d) = mapping.domains.read().unwrap().get(&domain_id)
|
||||||
|
&& let Some((&start, m)) = d.mappings.range(..=req.virt_end).next_back()
|
||||||
|
&& let Some(end) = inclusive_end(start, m.size)
|
||||||
|
&& end >= req.virt_start
|
||||||
|
{
|
||||||
|
status = VIRTIO_IOMMU_S_INVAL;
|
||||||
|
return Err(Error::InvalidMapRequest);
|
||||||
|
}
|
||||||
|
|
||||||
{
|
{
|
||||||
let domains = mapping.domains.read().unwrap();
|
let domains = mapping.domains.read().unwrap();
|
||||||
if let Some(d) = domains.get(&domain_id)
|
if let Some(d) = domains.get(&domain_id)
|
||||||
|
|||||||
Reference in New Issue
Block a user