Commit 8319e9d5 authored by Ard Biesheuvel's avatar Ard Biesheuvel Committed by Ingo Molnar
Browse files

efi/x86: Handle by-ref arguments covering multiple pages in mixed mode 



The mixed mode runtime wrappers are fragile when it comes to how the
memory referred to by its pointer arguments are laid out in memory, due
to the fact that it translates these addresses to physical addresses that
the runtime services can dereference when running in 1:1 mode. Since
vmalloc'ed pages (including the vmap'ed stack) are not contiguous in the
physical address space, this scheme only works if the referenced memory
objects do not cross page boundaries.

Currently, the mixed mode runtime service wrappers require that all by-ref
arguments that live in the vmalloc space have a size that is a power of 2,
and are aligned to that same value. While this is a sensible way to
construct an object that is guaranteed not to cross a page boundary, it is
overly strict when it comes to checking whether a given object violates
this requirement, as we can simply take the physical address of the first
and the last byte, and verify that they point into the same physical page.

When this check fails, we emit a WARN(), but then simply proceed with the
call, which could cause data corruption if the next physical page belongs
to a mapping that is entirely unrelated.

Given that with vmap'ed stacks, this condition is much more likely to
trigger, let's relax the condition a bit, but fail the runtime service
call if it does trigger.

Fixes: f6697df3 ("x86/efi: Prevent mixed mode boot corruption with CONFIG_VMAP_STACK=y")
Signed-off-by: default avatarArd Biesheuvel <ardb@kernel.org>
Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
Cc: linux-efi@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20200221084849.26878-4-ardb@kernel.org
parent f80c9f64

arch/x86/platform/efi/efi_64.c

+26 −19
Original line number Diff line number Diff line
@@ -180,7 +180,7 @@ void efi_sync_low_kernel_mappings(void) 
static inline phys_addr_t

virt_to_phys_or_null_size(void *va, unsigned long size)

{

	bool bad_size;

	phys_addr_t pa;



	if (!va)

		return 0;
@@ -188,16 +188,13 @@ virt_to_phys_or_null_size(void *va, unsigned long size) 
	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);

	pa = slow_virt_to_phys(va);



	WARN_ON(!IS_ALIGNED((unsigned long)va, size) || bad_size);

	/* check if the object crosses a page boundary */

	if (WARN_ON((pa ^ (pa + size - 1)) & PAGE_MASK))

		return 0;



	return slow_virt_to_phys(va);

	return pa;

}



#define virt_to_phys_or_null(addr)				\
@@ -615,6 +612,9 @@ efi_thunk_get_variable(efi_char16_t *name, efi_guid_t *vendor, 
	phys_attr = virt_to_phys_or_null(attr);

	phys_data = virt_to_phys_or_null_size(data, *data_size);



	if (!phys_name || (data && !phys_data))

		status = EFI_INVALID_PARAMETER;

	else

		status = efi_thunk(get_variable, phys_name, phys_vendor,

				   phys_attr, phys_data_size, phys_data);
@@ -641,7 +641,9 @@ efi_thunk_set_variable(efi_char16_t *name, efi_guid_t *vendor, 
	phys_vendor = virt_to_phys_or_null(vnd);

	phys_data = virt_to_phys_or_null_size(data, data_size);



	/* If data_size is > sizeof(u32) we've got problems */

	if (!phys_name || !phys_data)

		status = EFI_INVALID_PARAMETER;

	else

		status = efi_thunk(set_variable, phys_name, phys_vendor,

				   attr, data_size, phys_data);
@@ -670,7 +672,9 @@ efi_thunk_set_variable_nonblocking(efi_char16_t *name, efi_guid_t *vendor, 
	phys_vendor = virt_to_phys_or_null(vnd);

	phys_data = virt_to_phys_or_null_size(data, data_size);



	/* If data_size is > sizeof(u32) we've got problems */

	if (!phys_name || !phys_data)

		status = EFI_INVALID_PARAMETER;

	else

		status = efi_thunk(set_variable, phys_name, phys_vendor,

				   attr, data_size, phys_data);
@@ -698,6 +702,9 @@ efi_thunk_get_next_variable(unsigned long *name_size, 
	phys_vendor = virt_to_phys_or_null(vnd);

	phys_name = virt_to_phys_or_null_size(name, *name_size);



	if (!phys_name)

		status = EFI_INVALID_PARAMETER;

	else

		status = efi_thunk(get_next_variable, phys_name_size,

				   phys_name, phys_vendor);

CRA Git | Maintained and supported by SUSTech CRA and CCSE