Skip to content

Commit beca7e7

Browse files
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 <[email protected]>
1 parent a35528b commit beca7e7

File tree

2 files changed

+11
-18
lines changed

2 files changed

+11
-18
lines changed

src/aero_kernel/src/mem/paging/mapper.rs

-9
Original file line numberDiff line numberDiff line change
@@ -1128,15 +1128,6 @@ impl<'a> Translate for OffsetPageTable<'a> {
11281128
}
11291129

11301130
impl<'a> OffsetPageTable<'a> {
1131-
pub fn unmap_range(&mut self, range: Range<VirtAddr>, step: u64) -> Result<(), UnmapError> {
1132-
for addr in range.step_by(step as usize) {
1133-
let page: Page = Page::containing_address(addr);
1134-
self.inner.unmap(page)?.1.flush();
1135-
}
1136-
1137-
Ok(())
1138-
}
1139-
11401131
pub fn copy_page_range(&mut self, src: &mut OffsetPageTable, range: RangeInclusive<VirtAddr>) {
11411132
let mut map_to = |src: &mut OffsetPageTable, addr, frame, flags| match frame {
11421133
MappedFrame::Size4KiB(frame) => {

src/aero_kernel/src/userland/vm.rs

+11-9
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// along with Aero. If not, see <https://www.gnu.org/licenses/>.
1717

1818
use core::fmt::Write;
19+
use core::ops::Range;
1920

2021
use aero_syscall::{MMapFlags, MMapProt};
2122

@@ -733,16 +734,17 @@ impl Mapping {
733734
start: VirtAddr,
734735
end: VirtAddr,
735736
) -> Result<UnmapResult, UnmapError> {
736-
let mut unmap_range_inner = |range| -> Result<(), UnmapError> {
737-
match offset_table.unmap_range(range, Size4KiB::SIZE) {
738-
Ok(_) => Ok(()),
739-
740-
// its fine since technically we are not actually allocating the range
741-
// and they are just allocated on faults. So there might be a chance where we
742-
// try to unmap a region that is mapped but not actually allocated.
743-
Err(UnmapError::PageNotMapped) => Ok(()),
744-
Err(err) => Err(err),
737+
let mut unmap_range_inner = |range: Range<VirtAddr>| -> Result<(), UnmapError> {
738+
for addr in range.step_by(Size4KiB::SIZE as usize) {
739+
let page: Page = Page::containing_address(addr);
740+
match offset_table.unmap(page) {
741+
Ok((_, flusher)) => flusher.flush(),
742+
Err(UnmapError::PageNotMapped) => {}
743+
Err(e) => return Err(e),
744+
}
745745
}
746+
747+
Ok(())
746748
};
747749

748750
if end <= self.start_addr || start >= self.end_addr {

0 commit comments

Comments
 (0)