From 341826481aa4b14e61c77a0753d814c822133bdc Mon Sep 17 00:00:00 2001 From: Johannes Wikner Date: Tue, 29 Aug 2023 18:19:05 +0200 Subject: [PATCH] pagetables: allow user and kernel maps in same pgd KTF allows user mode to allocate high addresses, which normally are only available to kernel. However, to handle this somewhat gracefully, we need to check if already-existent page table protection bits conflicts with those of a new pgentry. This means that we may fixup NX, RW or USER flags if the new mapping is more permissive than current ones. This is a bit sketchy, yet useful for certains tests. Therefore, print a big warning if this ever happens. Signed-off-by: Johannes Wikner --- arch/x86/pagetables.c | 21 +++++++++++++++++++++ tests/unittests.c | 13 ++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/arch/x86/pagetables.c b/arch/x86/pagetables.c index 289a907f..7c7efa84 100644 --- a/arch/x86/pagetables.c +++ b/arch/x86/pagetables.c @@ -128,6 +128,23 @@ static mfn_t get_cr3_mfn(cr3_t *cr3_entry) { return cr3_entry->mfn; } +static inline void pgentry_fixup_flags(pgentry_t *entry, unsigned long flags) { + /* Our new flags will take precedence over previous one. */ + unsigned long entry_new = *entry; + entry_new |= flags & _PAGE_USER; + entry_new |= flags & _PAGE_RW; + entry_new &= ~(flags & _PAGE_NX); + if (unlikely(*entry != entry_new)) { + char flags_str_old[16]; + char flags_str_new[16]; + printk("WARNING: Already-present page table protection conflicts with our.\n" + " Updating present page table protection: %s -> %s\n", + dump_pte_flags(flags_str_old, 16, (pte_t) *entry), + dump_pte_flags(flags_str_new, 16, (pte_t) entry_new)); + *entry = entry_new; + } +} + static mfn_t get_pgentry_mfn(mfn_t tab_mfn, pt_index_t index, unsigned long flags) { pgentry_t *tab, *entry; mfn_t mfn; @@ -147,6 +164,10 @@ static mfn_t get_pgentry_mfn(mfn_t tab_mfn, pt_index_t index, unsigned long flag tab = init_map_mfn(mfn); memset(tab, 0, PAGE_SIZE); } + else { + /* Page table already exists but its flags may conflict with our. Maybe fixup */ + pgentry_fixup_flags(entry, flags); + } return mfn; } diff --git a/tests/unittests.c b/tests/unittests.c index 0c5c9fc0..8dc44434 100644 --- a/tests/unittests.c +++ b/tests/unittests.c @@ -33,6 +33,7 @@ #include #include +#include #include static char opt_string[4]; @@ -97,6 +98,12 @@ static unsigned long __user_text test_user_task_func3(void *arg) { return -16; } +#define HIGH_USER_PTR _ptr(0xffffffff80222000) +static unsigned long __user_text test_user_task_func4(void *arg) { + printf(USTR("access: %lx\n"), _ul(HIGH_USER_PTR)); + return *(unsigned long *) HIGH_USER_PTR; +} + int unit_tests(void *_unused) { printk("\nLet the UNITTESTs begin\n"); printk("Commandline parsing: %s\n", kernel_cmdline); @@ -165,13 +172,16 @@ int unit_tests(void *_unused) { cpu_freq_expect("Prototyp Amazing Foo One @ 1GHz", 1000000000); cpu_freq_expect("Prototyp Amazing Foo Two @ 1.00GHz", 1000000000); - task_t *task1, *task2, *task_user1, *task_user2, *task_user3; + task_t *task1, *task2, *task_user1, *task_user2, *task_user3, *task_user4; task1 = new_kernel_task("test1", test_kernel_task_func, _ptr(98)); task2 = new_kernel_task("test2", test_kernel_task_func, _ptr(-99)); task_user1 = new_user_task("test1 user", test_user_task_func1, NULL); task_user2 = new_user_task("test2 user", test_user_task_func2, NULL); task_user3 = new_user_task("test3 user", test_user_task_func3, NULL); + task_user4 = new_user_task("test4 user", test_user_task_func4, NULL); + + vmap_user_4k(HIGH_USER_PTR, get_free_frame()->mfn, L1_PROT_USER); set_task_repeat(task1, 10); schedule_task(task1, get_bsp_cpu()); @@ -179,6 +189,7 @@ int unit_tests(void *_unused) { schedule_task(task_user1, get_bsp_cpu()); schedule_task(task_user2, get_cpu(1)); schedule_task(task_user3, get_bsp_cpu()); + schedule_task(task_user4, get_cpu(1)); printk("Long mode to real mode transition:\n"); long_to_real();