| From f6697df36bdf0bf7fce984605c2918d4a7b4269f Mon Sep 17 00:00:00 2001 |
| From: Matt Fleming <matt@codeblueprint.co.uk> |
| Date: Sat, 12 Nov 2016 21:04:24 +0000 |
| Subject: [PATCH] x86/efi: Prevent mixed mode boot corruption with |
| CONFIG_VMAP_STACK=y |
| |
| commit f6697df36bdf0bf7fce984605c2918d4a7b4269f upstream. |
| |
| Booting an EFI mixed mode kernel has been crashing since commit: |
| |
| e37e43a497d5 ("x86/mm/64: Enable vmapped stacks (CONFIG_HAVE_ARCH_VMAP_STACK=y)") |
| |
| The user-visible effect in my test setup was the kernel being unable |
| to find the root file system ramdisk. This was likely caused by silent |
| memory or page table corruption. |
| |
| Enabling CONFIG_DEBUG_VIRTUAL=y immediately flagged the thunking code as |
| abusing virt_to_phys() because it was passing addresses that were not |
| part of the kernel direct mapping. |
| |
| Use the slow version instead, which correctly handles all memory |
| regions by performing a page table walk. |
| |
| Suggested-by: Andy Lutomirski <luto@amacapital.net> |
| Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk> |
| Cc: Andy Lutomirski <luto@kernel.org> |
| Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> |
| Cc: Borislav Petkov <bp@alien8.de> |
| Cc: Brian Gerst <brgerst@gmail.com> |
| Cc: Denys Vlasenko <dvlasenk@redhat.com> |
| Cc: H. Peter Anvin <hpa@zytor.com> |
| Cc: Josh Poimboeuf <jpoimboe@redhat.com> |
| Cc: Linus Torvalds <torvalds@linux-foundation.org> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Thomas Gleixner <tglx@linutronix.de> |
| Cc: linux-efi@vger.kernel.org |
| Link: http://lkml.kernel.org/r/20161112210424.5157-3-matt@codeblueprint.co.uk |
| Signed-off-by: Ingo Molnar <mingo@kernel.org> |
| |
| diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c |
| index 58b0f801f66f..319148bd4b05 100644 |
| --- a/arch/x86/platform/efi/efi_64.c |
| +++ b/arch/x86/platform/efi/efi_64.c |
| @@ -31,6 +31,7 @@ |
| #include <linux/io.h> |
| #include <linux/reboot.h> |
| #include <linux/slab.h> |
| +#include <linux/ucs2_string.h> |
| |
| #include <asm/setup.h> |
| #include <asm/page.h> |
| @@ -211,6 +212,35 @@ void efi_sync_low_kernel_mappings(void) |
| memcpy(pud_efi, pud_k, sizeof(pud_t) * num_entries); |
| } |
| |
| +/* |
| + * Wrapper for slow_virt_to_phys() that handles NULL addresses. |
| + */ |
| +static inline phys_addr_t |
| +virt_to_phys_or_null_size(void *va, unsigned long size) |
| +{ |
| + bool bad_size; |
| + |
| + if (!va) |
| + return 0; |
| + |
| + if (virt_addr_valid(va)) |
| + return virt_to_phys(va); |
| + |
| + /* |
| + * A fully aligned variable on the stack is guaranteed not to |
| + * cross a page bounary. Try to catch strings on the stack by |
| + * checking that 'size' is a power of two. |
| + */ |
| + bad_size = size > PAGE_SIZE || !is_power_of_2(size); |
| + |
| + WARN_ON(!IS_ALIGNED((unsigned long)va, size) || bad_size); |
| + |
| + return slow_virt_to_phys(va); |
| +} |
| + |
| +#define virt_to_phys_or_null(addr) \ |
| + virt_to_phys_or_null_size((addr), sizeof(*(addr))) |
| + |
| int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages) |
| { |
| unsigned long pfn, text; |
| @@ -494,8 +524,8 @@ static efi_status_t efi_thunk_get_time(efi_time_t *tm, efi_time_cap_t *tc) |
| |
| spin_lock(&rtc_lock); |
| |
| - phys_tm = virt_to_phys(tm); |
| - phys_tc = virt_to_phys(tc); |
| + phys_tm = virt_to_phys_or_null(tm); |
| + phys_tc = virt_to_phys_or_null(tc); |
| |
| status = efi_thunk(get_time, phys_tm, phys_tc); |
| |
| @@ -511,7 +541,7 @@ static efi_status_t efi_thunk_set_time(efi_time_t *tm) |
| |
| spin_lock(&rtc_lock); |
| |
| - phys_tm = virt_to_phys(tm); |
| + phys_tm = virt_to_phys_or_null(tm); |
| |
| status = efi_thunk(set_time, phys_tm); |
| |
| @@ -529,9 +559,9 @@ efi_thunk_get_wakeup_time(efi_bool_t *enabled, efi_bool_t *pending, |
| |
| spin_lock(&rtc_lock); |
| |
| - phys_enabled = virt_to_phys(enabled); |
| - phys_pending = virt_to_phys(pending); |
| - phys_tm = virt_to_phys(tm); |
| + phys_enabled = virt_to_phys_or_null(enabled); |
| + phys_pending = virt_to_phys_or_null(pending); |
| + phys_tm = virt_to_phys_or_null(tm); |
| |
| status = efi_thunk(get_wakeup_time, phys_enabled, |
| phys_pending, phys_tm); |
| @@ -549,7 +579,7 @@ efi_thunk_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm) |
| |
| spin_lock(&rtc_lock); |
| |
| - phys_tm = virt_to_phys(tm); |
| + phys_tm = virt_to_phys_or_null(tm); |
| |
| status = efi_thunk(set_wakeup_time, enabled, phys_tm); |
| |
| @@ -558,6 +588,10 @@ efi_thunk_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm) |
| return status; |
| } |
| |
| +static unsigned long efi_name_size(efi_char16_t *name) |
| +{ |
| + return ucs2_strsize(name, EFI_VAR_NAME_LEN) + 1; |
| +} |
| |
| static efi_status_t |
| efi_thunk_get_variable(efi_char16_t *name, efi_guid_t *vendor, |
| @@ -567,11 +601,11 @@ efi_thunk_get_variable(efi_char16_t *name, efi_guid_t *vendor, |
| u32 phys_name, phys_vendor, phys_attr; |
| u32 phys_data_size, phys_data; |
| |
| - phys_data_size = virt_to_phys(data_size); |
| - phys_vendor = virt_to_phys(vendor); |
| - phys_name = virt_to_phys(name); |
| - phys_attr = virt_to_phys(attr); |
| - phys_data = virt_to_phys(data); |
| + phys_data_size = virt_to_phys_or_null(data_size); |
| + phys_vendor = virt_to_phys_or_null(vendor); |
| + phys_name = virt_to_phys_or_null_size(name, efi_name_size(name)); |
| + phys_attr = virt_to_phys_or_null(attr); |
| + phys_data = virt_to_phys_or_null_size(data, *data_size); |
| |
| status = efi_thunk(get_variable, phys_name, phys_vendor, |
| phys_attr, phys_data_size, phys_data); |
| @@ -586,9 +620,9 @@ efi_thunk_set_variable(efi_char16_t *name, efi_guid_t *vendor, |
| u32 phys_name, phys_vendor, phys_data; |
| efi_status_t status; |
| |
| - phys_name = virt_to_phys(name); |
| - phys_vendor = virt_to_phys(vendor); |
| - phys_data = virt_to_phys(data); |
| + phys_name = virt_to_phys_or_null_size(name, efi_name_size(name)); |
| + phys_vendor = virt_to_phys_or_null(vendor); |
| + phys_data = virt_to_phys_or_null_size(data, data_size); |
| |
| /* If data_size is > sizeof(u32) we've got problems */ |
| status = efi_thunk(set_variable, phys_name, phys_vendor, |
| @@ -605,9 +639,9 @@ efi_thunk_get_next_variable(unsigned long *name_size, |
| efi_status_t status; |
| u32 phys_name_size, phys_name, phys_vendor; |
| |
| - phys_name_size = virt_to_phys(name_size); |
| - phys_vendor = virt_to_phys(vendor); |
| - phys_name = virt_to_phys(name); |
| + phys_name_size = virt_to_phys_or_null(name_size); |
| + phys_vendor = virt_to_phys_or_null(vendor); |
| + phys_name = virt_to_phys_or_null_size(name, *name_size); |
| |
| status = efi_thunk(get_next_variable, phys_name_size, |
| phys_name, phys_vendor); |
| @@ -621,7 +655,7 @@ efi_thunk_get_next_high_mono_count(u32 *count) |
| efi_status_t status; |
| u32 phys_count; |
| |
| - phys_count = virt_to_phys(count); |
| + phys_count = virt_to_phys_or_null(count); |
| status = efi_thunk(get_next_high_mono_count, phys_count); |
| |
| return status; |
| @@ -633,7 +667,7 @@ efi_thunk_reset_system(int reset_type, efi_status_t status, |
| { |
| u32 phys_data; |
| |
| - phys_data = virt_to_phys(data); |
| + phys_data = virt_to_phys_or_null_size(data, data_size); |
| |
| efi_thunk(reset_system, reset_type, status, data_size, phys_data); |
| } |
| @@ -661,9 +695,9 @@ efi_thunk_query_variable_info(u32 attr, u64 *storage_space, |
| if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION) |
| return EFI_UNSUPPORTED; |
| |
| - phys_storage = virt_to_phys(storage_space); |
| - phys_remaining = virt_to_phys(remaining_space); |
| - phys_max = virt_to_phys(max_variable_size); |
| + phys_storage = virt_to_phys_or_null(storage_space); |
| + phys_remaining = virt_to_phys_or_null(remaining_space); |
| + phys_max = virt_to_phys_or_null(max_variable_size); |
| |
| status = efi_thunk(query_variable_info, attr, phys_storage, |
| phys_remaining, phys_max); |
| -- |
| 2.15.0 |
| |