From beca7e7d7d6dba32d0fa8592c206f6ad753b551c Mon Sep 17 00:00:00 2001
From: Anhad Singh <andypythonappdeveloper@gmail.com>
Date: Wed, 3 Jul 2024 17:05:07 +1000
Subject: [PATCH] fix(vm): fix unmap silently not fully unmapping

Bug description:
offset.unmap_range() bailed out if unmap() internally returned an error.
The original intention was that the caller may choose to ignore the page
not mapped errors as of demand paging. This obviously due to bad API
design decision bailed out early instead of unmapping the rest of the
address range aswell. Causing all sorts of bugs. Now its fixed, yay!
(xfe also works now)

Signed-off-by: Anhad Singh <andypythonappdeveloper@gmail.com>
---
 src/aero_kernel/src/mem/paging/mapper.rs |  9 ---------
 src/aero_kernel/src/userland/vm.rs       | 20 +++++++++++---------
 2 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/src/aero_kernel/src/mem/paging/mapper.rs b/src/aero_kernel/src/mem/paging/mapper.rs
index 715ba1af0e3..2c218a5e5d3 100644
--- a/src/aero_kernel/src/mem/paging/mapper.rs
+++ b/src/aero_kernel/src/mem/paging/mapper.rs
@@ -1128,15 +1128,6 @@ impl<'a> Translate for OffsetPageTable<'a> {
 }
 
 impl<'a> OffsetPageTable<'a> {
-    pub fn unmap_range(&mut self, range: Range<VirtAddr>, step: u64) -> Result<(), UnmapError> {
-        for addr in range.step_by(step as usize) {
-            let page: Page = Page::containing_address(addr);
-            self.inner.unmap(page)?.1.flush();
-        }
-
-        Ok(())
-    }
-
     pub fn copy_page_range(&mut self, src: &mut OffsetPageTable, range: RangeInclusive<VirtAddr>) {
         let mut map_to = |src: &mut OffsetPageTable, addr, frame, flags| match frame {
             MappedFrame::Size4KiB(frame) => {
diff --git a/src/aero_kernel/src/userland/vm.rs b/src/aero_kernel/src/userland/vm.rs
index 92ebb78032a..8251431e557 100644
--- a/src/aero_kernel/src/userland/vm.rs
+++ b/src/aero_kernel/src/userland/vm.rs
@@ -16,6 +16,7 @@
 // along with Aero. If not, see <https://www.gnu.org/licenses/>.
 
 use core::fmt::Write;
+use core::ops::Range;
 
 use aero_syscall::{MMapFlags, MMapProt};
 
@@ -733,16 +734,17 @@ impl Mapping {
         start: VirtAddr,
         end: VirtAddr,
     ) -> Result<UnmapResult, UnmapError> {
-        let mut unmap_range_inner = |range| -> Result<(), UnmapError> {
-            match offset_table.unmap_range(range, Size4KiB::SIZE) {
-                Ok(_) => Ok(()),
-
-                // its fine since technically we are not actually allocating the range
-                // and they are just allocated on faults. So there might be a chance where we
-                // try to unmap a region that is mapped but not actually allocated.
-                Err(UnmapError::PageNotMapped) => Ok(()),
-                Err(err) => Err(err),
+        let mut unmap_range_inner = |range: Range<VirtAddr>| -> Result<(), UnmapError> {
+            for addr in range.step_by(Size4KiB::SIZE as usize) {
+                let page: Page = Page::containing_address(addr);
+                match offset_table.unmap(page) {
+                    Ok((_, flusher)) => flusher.flush(),
+                    Err(UnmapError::PageNotMapped) => {}
+                    Err(e) => return Err(e),
+                }
             }
+
+            Ok(())
         };
 
         if end <= self.start_addr || start >= self.end_addr {