commit da65ab179fa9b3072f87ff984dfa0d99cbc4c4e6
author Andrew Walbran <qwandor@google.com> Wed Dec 07 15:10:13 2022 +0000
committer Andrew Walbran <qwandor@google.com> Thu Dec 08 10:53:47 2022 +0000
tree 4644752c87af9cee2c2ca291e0facc3e91834104
parent 2dd64b398534b83022626e8de7bf759e436080f8

Check regions before mapping them.

If the checks fail, the region should not be mapped. Likewise if mapping
a region fails, it should not be added to the list of regions.

Test: Ran pVM firmware manually.
Change-Id: I231374108eaf719569cdd7bfb66ae347530fe379

pvmfw/src/memory.rs

diff --git a/pvmfw/src/memory.rs b/pvmfw/src/memory.rs
index ca0b886..e88fa5b 100644
--- a/pvmfw/src/memory.rs
+++ b/pvmfw/src/memory.rs
@@ -134,20 +134,24 @@
 
     /// Allocate the address range for a const slice; returns None if failed.
     pub fn alloc_range(&mut self, range: &MemoryRange) -> Result<MemoryRange> {
+        let region = MemoryRegion { range: range.clone(), mem_type: MemoryType::ReadOnly };
+        self.check(&region)?;
         self.page_table.map_rodata(range).map_err(|e| {
             error!("Error during range allocation: {e}");
             MemoryTrackerError::FailedToMap
         })?;
-        self.add(MemoryRegion { range: range.clone(), mem_type: MemoryType::ReadOnly })
+        self.add(region)
     }
 
     /// Allocate the address range for a mutable slice; returns None if failed.
     pub fn alloc_range_mut(&mut self, range: &MemoryRange) -> Result<MemoryRange> {
+        let region = MemoryRegion { range: range.clone(), mem_type: MemoryType::ReadWrite };
+        self.check(&region)?;
         self.page_table.map_data(range).map_err(|e| {
             error!("Error during mutable range allocation: {e}");
             MemoryTrackerError::FailedToMap
         })?;
-        self.add(MemoryRegion { range: range.clone(), mem_type: MemoryType::ReadWrite })
+        self.add(region)
     }
 
     /// Allocate the address range for a const slice; returns None if failed.
@@ -160,13 +164,23 @@
         self.alloc_range_mut(&(base..(base + size.get())))
     }
 
-    fn add(&mut self, region: MemoryRegion) -> Result<MemoryRange> {
+    /// Checks that the given region is within the range of the `MemoryTracker` and doesn't overlap
+    /// with any other previously allocated regions, and that the regions ArrayVec has capacity to
+    /// add it.
+    fn check(&self, region: &MemoryRegion) -> Result<()> {
         if !region.is_within(&self.total) {
             return Err(MemoryTrackerError::OutOfRange);
         }
-        if self.regions.iter().any(|r| r.overlaps(region.as_ref())) {
+        if self.regions.iter().any(|r| r.overlaps(&region.range)) {
             return Err(MemoryTrackerError::Overlaps);
         }
+        if self.regions.len() == self.regions.capacity() {
+            return Err(MemoryTrackerError::Full);
+        }
+        Ok(())
+    }
+
+    fn add(&mut self, region: MemoryRegion) -> Result<MemoryRange> {
         if self.regions.try_push(region).is_some() {
             return Err(MemoryTrackerError::Full);
         }