PageTable is not mapped as writable

[Wasabi375]

Some frames in the kernel page table are not mapped as writable. I'm not sure that is intended.

For example when the gdt is identity mapped the frames that are used in the page table are mapped as read only.
The reason is that Mapper::map_to only maps the page table frames as writable if the mapped frame is also writable.

https://docs.rs/x86_64/latest/x86_64/structures/paging/mapper/trait.Mapper.html#method.map_to

I don't think there is any reason why the kernel page table should ever be mapped as read only.
The question is if this is a bug in bootloader or if this should be addressed in the x86_64 crate.

[Wasabi375]

Maybe it's worth thinking about specifying how the kernel page table should be mapped. E.g. should we map all frames as read | write | no_exec or just read | write, etc.

[Freax13]

For example when the gdt is identity mapped the frames that are used in the page table are mapped as read only. The reason is that Mapper::map_to only maps the page table frames as writable if the mapped frame is also writable.

https://docs.rs/x86_64/latest/x86_64/structures/paging/mapper/trait.Mapper.html#method.map_to

I don't think there is any reason why the kernel page table should ever be mapped as read only. The question is if this is a bug in bootloader or if this should be addressed in the x86_64 crate.

This is conflating two different concepts: The WRITE bits in the higher-level page tables for a page and the writability of the mappings to these page tables. Omitting the WRITE bit from higher-level page tables has valid use cases so identity_map not setting the WRITE bits in the higher levels is not a bug. Regarding the writability of the mappings for the page tables, it all depends on the page table implementation. The bootloader supports recursive page tables and offset-mapping page tables. For recursive page tables, the higher-level entries have to be writeable and so that's what x86_64 does. For offset-mapping page tables, the offset-mapping is always writeable and so, again, the mappings to the page tables are writeable.

Some frames in the kernel page table are not mapped as writable. I'm not sure that is intended.

AFAICT there are two cases where frames are identity mapped, when mapping the GDT and when mapping the context switch function. There is an argument to be made whether the GDT should be writeable (though users can also just use their own GDT), but the context switch function definitively shouldn't be writeable (we should stick to W^X where possible).

What problem did you hit that prompted you to look into this?

[Wasabi375]

I'm using the recursive page table I get from the bootloader. I don't have a linear mapping.
In my case I get a page fault (caused by write) when mapping a new page.

Here are the relevant log statements from my kenrel:

DEBUG [::mem::page_table::test] Map page Page[4KiB](0xbe298000)
TRACE [::mem::page_table_debug_ext] Page Table entries for 0xbe298000: Entries before mapping
TRACE [::mem::page_table_debug_ext] L4 Entry: Table: Vaddr 0x20100804000:   Frame 0x67000 | Entry-Flags: PageTableFlags(PRESENT | WRITABLE | ACCESSED | DIRTY)
TRACE [::mem::page_table_debug_ext] L3 Entry: Table: Vaddr 0x20100800000:   Frame 0x68000 | Entry-Flags: PageTableFlags(PRESENT | ACCESSED)
TRACE [::mem::page_table_debug_ext] L2 Entry: Table: Vaddr 0x20100002000:   Frame 0x69000 | Entry-Flags: PageTableFlags(PRESENT | ACCESSED)
TRACE [::mem::page_table_debug_ext] L1 Entry: Table: Vaddr 0x200005f1000 | Entry-Flags: PageTableFlags(0x0)

TRACE [::mem::page_table_debug_ext] P4 Table: PageTable at 0x20100804000: Frame: 0x2000, flags: PageTableFlags(PRESENT | WRITABLE | ACCESSED | DIRTY | NO_EXECUTE)
TRACE [::mem::page_table_debug_ext] [0]: PageTableEntry: 0x67000, PageTableFlags(PRESENT | WRITABLE | ACCESSED | DIRTY)
TRACE [::mem::page_table_debug_ext] [1]: PageTableEntry: 0x3000, PageTableFlags(PRESENT | WRITABLE | ACCESSED)
TRACE [::mem::page_table_debug_ext] [2]: PageTableEntry: 0x51000, PageTableFlags(PRESENT | WRITABLE | ACCESSED)
TRACE [::mem::page_table_debug_ext] [3]: PageTableEntry: 0x6d000, PageTableFlags(PRESENT | WRITABLE | ACCESSED)
TRACE [::mem::page_table_debug_ext] [4]: PageTableEntry: 0x2000, PageTableFlags(PRESENT | WRITABLE | ACCESSED | DIRTY | NO_EXECUTE)
TRACE [::mem::page_table_debug_ext] [5]: PageTableEntry: 0x72000, PageTableFlags(PRESENT | WRITABLE | ACCESSED)

TRACE [::mem::page_table_debug_ext] P3 Table: PageTable at 0x20100800000: Frame: 0x67000, flags: PageTableFlags(PRESENT | WRITABLE | ACCESSED | DIRTY)
TRACE [::mem::page_table_debug_ext] [0]: PageTableEntry: 0x6b000, PageTableFlags(PRESENT | WRITABLE | ACCESSED | DIRTY)
TRACE [::mem::page_table_debug_ext] [2]: PageTableEntry: 0x68000, PageTableFlags(PRESENT | ACCESSED)

TRACE [::mem::page_table_debug_ext] P2 Table: PageTable at 0x20100002000: Frame: 0x68000, flags: PageTableFlags(PRESENT | ACCESSED)
TRACE [::mem::page_table_debug_ext] [497]: PageTableEntry: 0x69000, PageTableFlags(PRESENT | ACCESSED)

TRACE [::mem::page_table_debug_ext] P1 Table: PageTable at 0x200005f1000: Frame: 0x69000, flags: PageTableFlags(PRESENT | ACCESSED)
TRACE [::mem::page_table_debug_ext] [94]: PageTableEntry: 0xbe25e000, PageTableFlags(PRESENT | ACCESSED)
TRACE [::mem::page_table_debug_ext] [95]: PageTableEntry: 0xbe25f000, PageTableFlags(PRESENT)

WARN  [::cpu::int::default_handlers] PAGE FAULT: 0x200005f14c0: frame: Size4KiB(PhysFrame[4KiB](0x69000)), offset: 0x4C0, flags: PageTableFlags(PRESENT | ACCESSED)
ERROR [::testing::panic] TEST PANIC: panicked at wasabi-kernel/src/cpu/interrupts/default_handlers.rs:53:5:
PAGE FAULT:
Accessed Address: 0x200005f14c0
Error code: PageFaultErrorCode(PROTECTION_VIOLATION | CAUSED_BY_WRITE)

I use the recursive page table implementation from the x86_64 crate.
As you can see, the implementation is trying to write the L1 entry, but the page containing it is only mapped as readable.

This is conflating two different concepts: The WRITE bits in the higher-level page tables for a page and the writability of the mappings to these page tables. Omitting the WRITE bit from higher-level page tables has valid use cases so identity_map not setting the WRITE bits in the higher levels is not a bug.

I think this is correct as long as you are using a linear mapped page table. In that case there are always 2 mappings for each page in the page table. The linear mapping used to update the page table and the mapping used to resolve an address.
However in a recursively mapped page table there is only a single mapping to the frames, that is used both for updating the page table and resolving a virtual address.

I already tested my fix in #444 and with it I no longer get a page fault

[Freax13]

Oh, wow, okay, now I see what you mean. I'm surprised this hasn't been caught before, good catch!

So in summary, the bug here is that the recursive page table implementation always assumes that all WRITE bits in the higher-level page tables are set. The problem is that the bootloader uses linear mapped page tables during setup which works for the most part except that that implementation doesn't always set the WRITE bits if not needed.

[Wasabi375]

Oh, wow, okay, now I see what you mean. I'm surprised this hasn't been caught before, good catch!

I think it's mostly just random. The bootloader only maps a few pages as readonly. It just depends on how long it takes to randomly map a page that has an address that also hits one of those pages. That's a 1 in 512 chance. Sure that's only 2GB of memory but pages generally aren't completely chosen at random, so I'm not that surprised that most rust kernels don't hit this case.
I don't know how many kernels are written in rust that use recursive page tables, this bootloader and use a lot of memory, but I don't think there are many if any.
I guess I just got lucky 😉

[Freax13]

Fixed by #444