Commit f6697df3 authored by Matt Fleming's avatar Matt Fleming Committed by Ingo Molnar

x86/efi: Prevent mixed mode boot corruption with CONFIG_VMAP_STACK=y

Booting an EFI mixed mode kernel has been crashing since commit:

  e37e43a4 ("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: default avatarAndy Lutomirski <luto@amacapital.net>
Signed-off-by: default avatarMatt 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.ukSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 02e56902
...@@ -31,6 +31,7 @@ ...@@ -31,6 +31,7 @@
#include <linux/io.h> #include <linux/io.h>
#include <linux/reboot.h> #include <linux/reboot.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/ucs2_string.h>
#include <asm/setup.h> #include <asm/setup.h>
#include <asm/page.h> #include <asm/page.h>
...@@ -211,6 +212,35 @@ void efi_sync_low_kernel_mappings(void) ...@@ -211,6 +212,35 @@ void efi_sync_low_kernel_mappings(void)
memcpy(pud_efi, pud_k, sizeof(pud_t) * num_entries); 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) int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
{ {
unsigned long pfn, text; 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) ...@@ -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); spin_lock(&rtc_lock);
phys_tm = virt_to_phys(tm); phys_tm = virt_to_phys_or_null(tm);
phys_tc = virt_to_phys(tc); phys_tc = virt_to_phys_or_null(tc);
status = efi_thunk(get_time, phys_tm, phys_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) ...@@ -511,7 +541,7 @@ static efi_status_t efi_thunk_set_time(efi_time_t *tm)
spin_lock(&rtc_lock); 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); status = efi_thunk(set_time, phys_tm);
...@@ -529,9 +559,9 @@ efi_thunk_get_wakeup_time(efi_bool_t *enabled, efi_bool_t *pending, ...@@ -529,9 +559,9 @@ efi_thunk_get_wakeup_time(efi_bool_t *enabled, efi_bool_t *pending,
spin_lock(&rtc_lock); spin_lock(&rtc_lock);
phys_enabled = virt_to_phys(enabled); phys_enabled = virt_to_phys_or_null(enabled);
phys_pending = virt_to_phys(pending); phys_pending = virt_to_phys_or_null(pending);
phys_tm = virt_to_phys(tm); phys_tm = virt_to_phys_or_null(tm);
status = efi_thunk(get_wakeup_time, phys_enabled, status = efi_thunk(get_wakeup_time, phys_enabled,
phys_pending, phys_tm); phys_pending, phys_tm);
...@@ -549,7 +579,7 @@ efi_thunk_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm) ...@@ -549,7 +579,7 @@ efi_thunk_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
spin_lock(&rtc_lock); 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); 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) ...@@ -558,6 +588,10 @@ efi_thunk_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
return status; 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 static efi_status_t
efi_thunk_get_variable(efi_char16_t *name, efi_guid_t *vendor, 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, ...@@ -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_name, phys_vendor, phys_attr;
u32 phys_data_size, phys_data; u32 phys_data_size, phys_data;
phys_data_size = virt_to_phys(data_size); phys_data_size = virt_to_phys_or_null(data_size);
phys_vendor = virt_to_phys(vendor); phys_vendor = virt_to_phys_or_null(vendor);
phys_name = virt_to_phys(name); phys_name = virt_to_phys_or_null_size(name, efi_name_size(name));
phys_attr = virt_to_phys(attr); phys_attr = virt_to_phys_or_null(attr);
phys_data = virt_to_phys(data); phys_data = virt_to_phys_or_null_size(data, *data_size);
status = efi_thunk(get_variable, phys_name, phys_vendor, status = efi_thunk(get_variable, phys_name, phys_vendor,
phys_attr, phys_data_size, phys_data); phys_attr, phys_data_size, phys_data);
...@@ -586,9 +620,9 @@ efi_thunk_set_variable(efi_char16_t *name, efi_guid_t *vendor, ...@@ -586,9 +620,9 @@ efi_thunk_set_variable(efi_char16_t *name, efi_guid_t *vendor,
u32 phys_name, phys_vendor, phys_data; u32 phys_name, phys_vendor, phys_data;
efi_status_t status; efi_status_t status;
phys_name = virt_to_phys(name); phys_name = virt_to_phys_or_null_size(name, efi_name_size(name));
phys_vendor = virt_to_phys(vendor); phys_vendor = virt_to_phys_or_null(vendor);
phys_data = virt_to_phys(data); phys_data = virt_to_phys_or_null_size(data, data_size);
/* If data_size is > sizeof(u32) we've got problems */ /* If data_size is > sizeof(u32) we've got problems */
status = efi_thunk(set_variable, phys_name, phys_vendor, status = efi_thunk(set_variable, phys_name, phys_vendor,
...@@ -605,9 +639,9 @@ efi_thunk_get_next_variable(unsigned long *name_size, ...@@ -605,9 +639,9 @@ efi_thunk_get_next_variable(unsigned long *name_size,
efi_status_t status; efi_status_t status;
u32 phys_name_size, phys_name, phys_vendor; u32 phys_name_size, phys_name, phys_vendor;
phys_name_size = virt_to_phys(name_size); phys_name_size = virt_to_phys_or_null(name_size);
phys_vendor = virt_to_phys(vendor); phys_vendor = virt_to_phys_or_null(vendor);
phys_name = virt_to_phys(name); phys_name = virt_to_phys_or_null_size(name, *name_size);
status = efi_thunk(get_next_variable, phys_name_size, status = efi_thunk(get_next_variable, phys_name_size,
phys_name, phys_vendor); phys_name, phys_vendor);
...@@ -621,7 +655,7 @@ efi_thunk_get_next_high_mono_count(u32 *count) ...@@ -621,7 +655,7 @@ efi_thunk_get_next_high_mono_count(u32 *count)
efi_status_t status; efi_status_t status;
u32 phys_count; 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); status = efi_thunk(get_next_high_mono_count, phys_count);
return status; return status;
...@@ -633,7 +667,7 @@ efi_thunk_reset_system(int reset_type, efi_status_t status, ...@@ -633,7 +667,7 @@ efi_thunk_reset_system(int reset_type, efi_status_t status,
{ {
u32 phys_data; 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); 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, ...@@ -661,9 +695,9 @@ efi_thunk_query_variable_info(u32 attr, u64 *storage_space,
if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION) if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
return EFI_UNSUPPORTED; return EFI_UNSUPPORTED;
phys_storage = virt_to_phys(storage_space); phys_storage = virt_to_phys_or_null(storage_space);
phys_remaining = virt_to_phys(remaining_space); phys_remaining = virt_to_phys_or_null(remaining_space);
phys_max = virt_to_phys(max_variable_size); phys_max = virt_to_phys_or_null(max_variable_size);
status = efi_thunk(query_variable_info, attr, phys_storage, status = efi_thunk(query_variable_info, attr, phys_storage,
phys_remaining, phys_max); phys_remaining, phys_max);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment