Skip to content

Commit

Permalink
fix(vm): fix unmap silently not fully unmapping
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Andy-Python-Programmer committed Jul 3, 2024
1 parent a35528b commit beca7e7
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 18 deletions.
9 changes: 0 additions & 9 deletions src/aero_kernel/src/mem/paging/mapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
20 changes: 11 additions & 9 deletions src/aero_kernel/src/userland/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit beca7e7

Please sign in to comment.