From f8a629cce69fdd31be2dd427da04727f60817269 Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Fri, 24 May 2024 13:28:44 +0200 Subject: [PATCH] Ensure all page table frames are mapped as writable --- Changelog.md | 2 ++ common/src/lib.rs | 23 +++++++++++++++++++++-- common/src/load_kernel.rs | 22 ++++++++++++++++++++-- 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/Changelog.md b/Changelog.md index 3ae96afd..6ac95ddc 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,7 @@ # Unreleased +* Fix bug leading to page table frames that are not mapped as writable + # 0.11.7 – 2024-02-16 * Set `NO_EXECUTE` flag for all writable memory regions by @phil-opp in https://github.com/rust-osdev/bootloader/pull/409 diff --git a/common/src/lib.rs b/common/src/lib.rs index 3533e0fe..c17f68fc 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -241,8 +241,18 @@ where context_switch_function_start_frame, context_switch_function_start_frame + 1, ) { + let page = Page::containing_address(VirtAddr::new(frame.start_address().as_u64())); match unsafe { - kernel_page_table.identity_map(frame, PageTableFlags::PRESENT, frame_allocator) + // The parent table flags need to be both readable and writable to + // support recursive page tables. + // See https://github.com/rust-osdev/bootloader/issues/443#issuecomment-2130010621 + kernel_page_table.map_to_with_table_flags( + page, + frame, + PageTableFlags::PRESENT, + PageTableFlags::PRESENT | PageTableFlags::WRITABLE, + frame_allocator, + ) } { Ok(tlb) => tlb.flush(), Err(err) => panic!("failed to identity map frame {:?}: {:?}", frame, err), @@ -254,8 +264,17 @@ where .allocate_frame() .expect("failed to allocate GDT frame"); gdt::create_and_load(gdt_frame); + let gdt_page = Page::containing_address(VirtAddr::new(gdt_frame.start_address().as_u64())); match unsafe { - kernel_page_table.identity_map(gdt_frame, PageTableFlags::PRESENT, frame_allocator) + // The parent table flags need to be both readable and writable to + // support recursive page tables. + kernel_page_table.map_to_with_table_flags( + gdt_page, + gdt_frame, + PageTableFlags::PRESENT, + PageTableFlags::PRESENT | PageTableFlags::WRITABLE, + frame_allocator, + ) } { Ok(tlb) => tlb.flush(), Err(err) => panic!("failed to identity map frame {:?}: {:?}", gdt_frame, err), diff --git a/common/src/load_kernel.rs b/common/src/load_kernel.rs index 68ad9778..6cc24fed 100644 --- a/common/src/load_kernel.rs +++ b/common/src/load_kernel.rs @@ -185,8 +185,17 @@ where let offset = frame - start_frame; let page = start_page + offset; let flusher = unsafe { + // The parent table flags need to be both readable and writable to + // support recursive page tables. + // See https://github.com/rust-osdev/bootloader/issues/443#issuecomment-2130010621 self.page_table - .map_to(page, frame, segment_flags, self.frame_allocator) + .map_to_with_table_flags( + page, + frame, + segment_flags, + Flags::PRESENT | Flags::WRITABLE, + self.frame_allocator, + ) .map_err(|_err| "map_to failed")? }; // we operate on an inactive page table, so there's no need to flush anything @@ -280,8 +289,17 @@ where // map frame let flusher = unsafe { + // The parent table flags need to be both readable and writable to + // support recursive page tables. + // See https://github.com/rust-osdev/bootloader/issues/443#issuecomment-2130010621 self.page_table - .map_to(page, frame, segment_flags, self.frame_allocator) + .map_to_with_table_flags( + page, + frame, + segment_flags, + Flags::PRESENT | Flags::WRITABLE, + self.frame_allocator, + ) .map_err(|_err| "Failed to map new frame for bss memory")? }; // we operate on an inactive page table, so we don't need to flush our changes