diff --git a/virtio-devices/src/device.rs b/virtio-devices/src/device.rs index 2349db6b5..4c61ba35d 100644 --- a/virtio-devices/src/device.rs +++ b/virtio-devices/src/device.rs @@ -202,10 +202,23 @@ pub trait VirtioDevice: Send { /// On the other side, the implementation itself should be provided by the code /// emulating the IOMMU for the guest. pub trait DmaRemapping { - /// Provide a way to translate GVA address ranges into GPAs. - fn translate_gva(&self, id: u32, addr: u64) -> std::result::Result; - /// Provide a way to translate GPA address ranges into GVAs. - fn translate_gpa(&self, id: u32, addr: u64) -> std::result::Result; + /// Provide a way to translate GVA address ranges into GPAs. The + /// implementation must reject translations whose [addr, addr+size) + /// span isn't entirely covered by a single mapping. + fn translate_gva( + &self, + id: u32, + addr: u64, + size: u64, + ) -> std::result::Result; + /// Provide a way to translate GPA address ranges into GVAs. Same + /// span requirement as `translate_gva`. + fn translate_gpa( + &self, + id: u32, + addr: u64, + size: u64, + ) -> std::result::Result; } /// Structure to handle device state common to all devices diff --git a/virtio-devices/src/iommu.rs b/virtio-devices/src/iommu.rs index c3ad4a72a..10a1fc43e 100644 --- a/virtio-devices/src/iommu.rs +++ b/virtio-devices/src/iommu.rs @@ -819,9 +819,32 @@ pub struct IommuMapping { bypass: AtomicBool, } +// Inclusive end of `[addr, addr+size)`. Returns None for zero size or +// overflow so range checks cannot be bypassed. +fn inclusive_end(addr: u64, size: u64) -> Option { + if size == 0 { + return None; + } + addr.checked_add(size - 1) +} + +fn span_end(addr: u64, size: u64) -> std::result::Result { + inclusive_end(addr, size).ok_or_else(|| { + io::Error::other(format!( + "translate span overflow or zero size: addr 0x{addr:x} size 0x{size:x}" + )) + }) +} + impl DmaRemapping for IommuMapping { - fn translate_gva(&self, id: u32, addr: u64) -> std::result::Result { - debug!("Translate GVA addr 0x{addr:x}"); + fn translate_gva( + &self, + id: u32, + addr: u64, + size: u64, + ) -> std::result::Result { + debug!("Translate GVA addr 0x{addr:x} size 0x{size:x}"); + let end = span_end(addr, size)?; if let Some(domain_id) = self.endpoints.read().unwrap().get(&id) { if let Some(domain) = self.domains.read().unwrap().get(domain_id) { // Directly return identity mapping in case the domain is in @@ -831,7 +854,10 @@ impl DmaRemapping for IommuMapping { } for (&key, &value) in domain.mappings.iter() { - if addr >= key && addr < key + value.size { + if let Some(mapping_end) = inclusive_end(key, value.size) + && addr >= key + && end <= mapping_end + { let new_addr = addr - key + value.gpa; debug!("Into GPA addr 0x{new_addr:x}"); return Ok(new_addr); @@ -843,12 +869,18 @@ impl DmaRemapping for IommuMapping { } Err(io::Error::other(format!( - "failed to translate GVA addr 0x{addr:x}" + "failed to translate GVA addr 0x{addr:x} size 0x{size:x}" ))) } - fn translate_gpa(&self, id: u32, addr: u64) -> std::result::Result { - debug!("Translate GPA addr 0x{addr:x}"); + fn translate_gpa( + &self, + id: u32, + addr: u64, + size: u64, + ) -> std::result::Result { + debug!("Translate GPA addr 0x{addr:x} size 0x{size:x}"); + let end = span_end(addr, size)?; if let Some(domain_id) = self.endpoints.read().unwrap().get(&id) { if let Some(domain) = self.domains.read().unwrap().get(domain_id) { // Directly return identity mapping in case the domain is in @@ -858,7 +890,10 @@ impl DmaRemapping for IommuMapping { } for (&key, &value) in domain.mappings.iter() { - if addr >= value.gpa && addr < value.gpa + value.size { + if let Some(mapping_gpa_end) = inclusive_end(value.gpa, value.size) + && addr >= value.gpa + && end <= mapping_gpa_end + { let new_addr = addr - value.gpa + key; debug!("Into GVA addr 0x{new_addr:x}"); return Ok(new_addr); @@ -870,7 +905,7 @@ impl DmaRemapping for IommuMapping { } Err(io::Error::other(format!( - "failed to translate GPA addr 0x{addr:x}" + "failed to translate GPA addr 0x{addr:x} size 0x{size:x}" ))) } } @@ -888,11 +923,11 @@ impl AccessPlatformMapping { } impl AccessPlatform for AccessPlatformMapping { - fn translate_gva(&self, base: u64, _size: u64) -> std::result::Result { - self.mapping.translate_gva(self.id, base) + fn translate_gva(&self, base: u64, size: u64) -> std::result::Result { + self.mapping.translate_gva(self.id, base, size) } - fn translate_gpa(&self, base: u64, _size: u64) -> std::result::Result { - self.mapping.translate_gpa(self.id, base) + fn translate_gpa(&self, base: u64, size: u64) -> std::result::Result { + self.mapping.translate_gpa(self.id, base, size) } }