RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-05-30 21:38:11 +00:00
Code Issues Activity Actions

Kernel: Prevent sign bit extension when creating a PDPTE

Browse source 
When doing the cast to u64 on the page directory physical address,
the sign bit was being extended. This only beomes an issue when
crossing the 2 GiB boundary. At >= 2 GiB, the physical address
has the sign bit set. For example, 0x80000000.

This set all the reserved bits in the PDPTE, causing a GPF
when loading the PDPT pointer into CR3. The reserved bits are
presumably there to stop you writing out a physical address that
the CPU physically cannot handle, as the size of the reserved bits
is determined by the physical address width of the CPU.

This fixes this by casting to FlatPtr instead. I believe the sign
extension only happens when casting to a bigger type. I'm also using
FlatPtr because it's a pointer we're writing into the PDPTE.
sizeof(FlatPtr) will always be the same size as sizeof(void*).

This also now asserts that the physical address in the PDPTE is
within the max physical address the CPU supports. This is better
than getting a GPF, because CPU::handle_crash tries to do the same
operation that caused the GPF in the first place. That would cause
an infinite loop of GPFs until the stack was exhausted, causing a
triple fault.

As far as I know and tested, I believe we can now use the full 32-bit
physical range without crashing.

Fixes #4584. See that issue for the full debugging story.
This commit is contained in:
Luke 2020-12-30 17:31:25 +00:00 • committed by Andreas Kling
parent d014277973
commit 865f5ed4f6
3 changed files with 44 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

34
Kernel/VM/PageDirectory.cpp
View file

@ -95,10 +95,36 @@ PageDirectory::PageDirectory(Process& process, const RangeAllocator* parent_rang
{
auto& table = *(PageDirectoryPointerTable*)MM.quickmap_page(*m_directory_table);
table.raw[0] = (u64)m_directory_pages[0]->paddr().as_ptr() | 1;
table.raw[1] = (u64)m_directory_pages[1]->paddr().as_ptr() | 1;
table.raw[2] = (u64)m_directory_pages[2]->paddr().as_ptr() | 1;
table.raw[3] = (u64)m_directory_pages[3]->paddr().as_ptr() | 1;
table.raw[0] = (FlatPtr)m_directory_pages[0]->paddr().as_ptr() | 1;
table.raw[1] = (FlatPtr)m_directory_pages[1]->paddr().as_ptr() | 1;
table.raw[2] = (FlatPtr)m_directory_pages[2]->paddr().as_ptr() | 1;
table.raw[3] = (FlatPtr)m_directory_pages[3]->paddr().as_ptr() | 1;
// 2 ** MAXPHYADDR - 1
// Where MAXPHYADDR = physical_address_bit_width
u64 max_physical_address = (1ULL << Processor::current().physical_address_bit_width()) - 1;
// bit 63 = no execute
// bit 7 = page size
// bit 5 = accessed
// bit 4 = cache disable
// bit 3 = write through
// bit 2 = user/supervisor
// bit 1 = read/write
// bit 0 = present
constexpr u64 pdpte_bit_flags = 0x80000000000000BF;
// This is to notify us of bugs where we're:
// 1. Going over what the processor is capable of.
// 2. Writing into the reserved bits (51:MAXPHYADDR), where doing so throws a GPF
// when writing out the PDPT pointer to CR3.
// The reason we're not checking the page directory's physical address directly is because
// we're checking for sign extension when putting it into a PDPTE. See issue #4584.
ASSERT((table.raw[0] & ~pdpte_bit_flags) <= max_physical_address);
ASSERT((table.raw[1] & ~pdpte_bit_flags) <= max_physical_address);
ASSERT((table.raw[2] & ~pdpte_bit_flags) <= max_physical_address);
ASSERT((table.raw[3] & ~pdpte_bit_flags) <= max_physical_address);
MM.unquickmap_page();
}