mirror of
https://github.com/cloud-hypervisor/cloud-hypervisor.git
synced 2026-08-05 02:19:16 +00:00
virtio-devices: iommu: Roll back partial MAP and fix DETACH panic
A concurrent DETACH between the read-lock check and the write-lock get_mut().unwrap() in MAP/UNMAP would panic the worker. Replace the unwrap with a let-else. A failure on a later endpoint in the per-endpoint MAP loop, or a DETACH that races the missing-domain branch, must roll back the external mappings already installed; otherwise domain.mappings diverges from VFIO state. Signed-off-by: Rob Bradford <rbradford@meta.com> Assisted-by: Claude:claude-opus-4-7
This commit is contained in:
@@ -121,7 +121,6 @@ const VIRTIO_IOMMU_S_OK: u8 = 0;
|
||||
const VIRTIO_IOMMU_S_IOERR: u8 = 1;
|
||||
#[allow(unused)]
|
||||
const VIRTIO_IOMMU_S_UNSUPP: u8 = 2;
|
||||
#[allow(unused)]
|
||||
const VIRTIO_IOMMU_S_DEVERR: u8 = 3;
|
||||
#[allow(unused)]
|
||||
const VIRTIO_IOMMU_S_INVAL: u8 = 4;
|
||||
@@ -510,32 +509,41 @@ impl Request {
|
||||
return Err(Error::InvalidMapRequest);
|
||||
}
|
||||
|
||||
let mut mapped: Vec<u32> = Vec::new();
|
||||
let rollback = |mapped: &[u32]| {
|
||||
for ep in mapped {
|
||||
if let Some(pmap) = ext_mapping.get(ep) {
|
||||
let _ = pmap.unmap(req.virt_start, size);
|
||||
}
|
||||
}
|
||||
};
|
||||
// For viommu all endpoints receive their own VFIO container, as a result
|
||||
// Each endpoint within the domain needs to be separately mapped, as the
|
||||
// mapping is done on a per-container level, not a per-domain level
|
||||
for endpoint in endpoints {
|
||||
if let Some(ext_map) = ext_mapping.get(&endpoint) {
|
||||
ext_map
|
||||
.map(req.virt_start, req.phys_start, size)
|
||||
.map_err(Error::ExternalMapping)?;
|
||||
if let Err(e) = ext_map.map(req.virt_start, req.phys_start, size) {
|
||||
rollback(&mapped);
|
||||
status = VIRTIO_IOMMU_S_DEVERR;
|
||||
return Err(Error::ExternalMapping(e));
|
||||
}
|
||||
mapped.push(endpoint);
|
||||
}
|
||||
}
|
||||
|
||||
// Add new mapping associated with the domain
|
||||
mapping
|
||||
.domains
|
||||
.write()
|
||||
.unwrap()
|
||||
.get_mut(&domain_id)
|
||||
.unwrap()
|
||||
.mappings
|
||||
.insert(
|
||||
req.virt_start,
|
||||
Mapping {
|
||||
gpa: req.phys_start,
|
||||
size,
|
||||
},
|
||||
);
|
||||
let mut domains = mapping.domains.write().unwrap();
|
||||
let Some(domain) = domains.get_mut(&domain_id) else {
|
||||
rollback(&mapped);
|
||||
status = VIRTIO_IOMMU_S_NOENT;
|
||||
return Err(Error::InvalidMapRequestMissingDomain);
|
||||
};
|
||||
domain.mappings.insert(
|
||||
req.virt_start,
|
||||
Mapping {
|
||||
gpa: req.phys_start,
|
||||
size,
|
||||
},
|
||||
);
|
||||
}
|
||||
VIRTIO_IOMMU_T_UNMAP => {
|
||||
if desc_size_left != size_of::<VirtioIommuReqUnmap>() {
|
||||
@@ -614,13 +622,12 @@ impl Request {
|
||||
}
|
||||
}
|
||||
|
||||
// Remove all mappings associated with the domain within the requested range
|
||||
mapping
|
||||
.domains
|
||||
.write()
|
||||
.unwrap()
|
||||
.get_mut(&domain_id)
|
||||
.unwrap()
|
||||
let mut domains = mapping.domains.write().unwrap();
|
||||
let Some(domain) = domains.get_mut(&domain_id) else {
|
||||
status = VIRTIO_IOMMU_S_INVAL;
|
||||
return Err(Error::InvalidUnmapRequestMissingDomain);
|
||||
};
|
||||
domain
|
||||
.mappings
|
||||
.retain(|&x, _| x < req.virt_start || x > req.virt_end);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user