mirror of
https://github.com/cloud-hypervisor/cloud-hypervisor.git
synced 2026-08-05 02:19:16 +00:00
virtio-devices: iommu: Pass size to translate_gva/translate_gpa
The DmaRemapping translate_gva and translate_gpa entry points discarded the size argument that AccessPlatform's signature already carries and only checked the base address. A buffer beginning inside a mapping but extending past it was treated as fully translated, allowing reads or writes outside the IOMMU-authorized window. Add `size` to the trait, validate the full span fits in a single mapping, and propagate it through AccessPlatformMapping. Signed-off-by: Rob Bradford <rbradford@meta.com> Assisted-by: Claude:claude-opus-4-7
This commit is contained in:
@@ -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<u64, std::io::Error>;
|
||||
/// Provide a way to translate GPA address ranges into GVAs.
|
||||
fn translate_gpa(&self, id: u32, addr: u64) -> std::result::Result<u64, std::io::Error>;
|
||||
/// 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<u64, std::io::Error>;
|
||||
/// 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<u64, std::io::Error>;
|
||||
}
|
||||
|
||||
/// Structure to handle device state common to all devices
|
||||
|
||||
@@ -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<u64> {
|
||||
if size == 0 {
|
||||
return None;
|
||||
}
|
||||
addr.checked_add(size - 1)
|
||||
}
|
||||
|
||||
fn span_end(addr: u64, size: u64) -> std::result::Result<u64, io::Error> {
|
||||
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<u64, std::io::Error> {
|
||||
debug!("Translate GVA addr 0x{addr:x}");
|
||||
fn translate_gva(
|
||||
&self,
|
||||
id: u32,
|
||||
addr: u64,
|
||||
size: u64,
|
||||
) -> std::result::Result<u64, std::io::Error> {
|
||||
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<u64, std::io::Error> {
|
||||
debug!("Translate GPA addr 0x{addr:x}");
|
||||
fn translate_gpa(
|
||||
&self,
|
||||
id: u32,
|
||||
addr: u64,
|
||||
size: u64,
|
||||
) -> std::result::Result<u64, std::io::Error> {
|
||||
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<u64, std::io::Error> {
|
||||
self.mapping.translate_gva(self.id, base)
|
||||
fn translate_gva(&self, base: u64, size: u64) -> std::result::Result<u64, std::io::Error> {
|
||||
self.mapping.translate_gva(self.id, base, size)
|
||||
}
|
||||
fn translate_gpa(&self, base: u64, _size: u64) -> std::result::Result<u64, std::io::Error> {
|
||||
self.mapping.translate_gpa(self.id, base)
|
||||
fn translate_gpa(&self, base: u64, size: u64) -> std::result::Result<u64, std::io::Error> {
|
||||
self.mapping.translate_gpa(self.id, base, size)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user