[U-Boot] [RFC 1/6] efi_loader: runtime: make SetVirtualAddressMap configurable

[Heinrich Schuchardt]

On 6/15/19 11:14 PM, Mark Kettenis wrote:
>> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> Date: Sat, 15 Jun 2019 21:46:02 +0200
>>
>> On 6/5/19 6:21 AM, AKASHI Takahiro wrote:
>>> OS does not always need to call SetVirtualAddressMap.
>>> (Ard confirmed this on arm64 linux.)
>>> So let this API configurable. If disabled, it will return EFI_UNSUPPORTED
>>> as UEFI specification requires.
>>
>> Currently we do not support this scenario. Alex's patch should go in first.
>
> OpenBSD/arm64 will always call this function.  It does this in order

OpenBSD/arm32 does not call it. I have no clue why the 32bit and 64bit
code have diverged.

> to randomize the virtual addresses used by runtime services to make it
> harder for an attacker to call into UEFI runtime services.  Note that
> the UEFI 2.7 standard provides no indication that this interface might
> be optional, so I don't think OpenBSD is doing anything wrong here.
>
> I think it is unwise to make this API configurable.  But disabling it
> by default like this diff does would be a seriously bad idea.  It
> means U-Boot would no longer be backwards compatible with UEFI 2.7.

The current UEFI specification is 2.8. It foresees that
SetVirtualAddressMap() can be marked as unsupported but that does not
imply that we have to make it customizable. And disabling it by default
does not make much sense while most operating systems use it.

Best regards

Heinrich

>
>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>> ---
>>>    lib/efi_loader/Kconfig       | 7 +++++++
>>>    lib/efi_loader/efi_runtime.c | 8 ++++++++
>>>    2 files changed, 15 insertions(+)
>>>
>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>>> index 8bf4b1754d06..bb9c7582b14d 100644
>>> --- a/lib/efi_loader/Kconfig
>>> +++ b/lib/efi_loader/Kconfig
>>> @@ -44,6 +44,13 @@ config EFI_SET_TIME
>>>    	  Provide the SetTime() runtime service at boottime. This service
>>>    	  can be used by an EFI application to adjust the real time clock.
>>>
>>> +config EFI_RUNTIME_SET_VIRTUAL_ADDRESS_MAP
>>> +	bool "runtime service: SetVirtualAddressMap"
>>> +	default n
>>> +	help
>>> +	  Enable SetVirtualAddressMap runtime service. This API will be
>>> +	  called by OS just before it enters into virtual address mode.
>>> +
>>>    config EFI_DEVICE_PATH_TO_TEXT
>>>    	bool "Device path to text protocol"
>>>    	default y
>>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>>> index 9c50955c9bd0..60442cb21d37 100644
>>> --- a/lib/efi_loader/efi_runtime.c
>>> +++ b/lib/efi_loader/efi_runtime.c
>>> @@ -374,10 +374,12 @@ static const struct efi_runtime_detach_list_struct efi_runtime_detach_list[] = {
>>>    		/* do_reset is gone */
>>>    		.ptr = &efi_runtime_services.reset_system,
>>>    		.patchto = efi_reset_system,
>>> +#ifdef CONFIG_RUNTIME_SET_VIRTUAL_ADDRESS_MAP
>>>    	}, {
>>>    		/* invalidate_*cache_all are gone */
>>>    		.ptr = &efi_runtime_services.set_virtual_address_map,
>>>    		.patchto = &efi_unimplemented,
>>> +#endif
>>>    	}, {
>>>    		/* RTC accessors are gone */
>>>    		.ptr = &efi_runtime_services.get_time,
>>> @@ -512,6 +514,7 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
>>>            invalidate_icache_all();
>>>    }
>>>
>>> +#ifdef CONFIG_RUNTIME_SET_VIRTUAL_ADDRESS_MAP
>>>    /**
>>>     * efi_set_virtual_address_map() - change from physical to virtual mapping
>>>     *
>>> @@ -619,6 +622,7 @@ static efi_status_t EFIAPI efi_set_virtual_address_map(
>>>
>>>    	return EFI_EXIT(EFI_INVALID_PARAMETER);
>>>    }
>>> +#endif /* CONFIG_RUNTIME_SET_VIRTUAL_ADDRESS_MAP */
>>>
>>>    /**
>>>     * efi_add_runtime_mmio() - add memory-mapped IO region
>>> @@ -796,7 +800,11 @@ struct efi_runtime_services __efi_runtime_data efi_runtime_services = {
>>>    	.set_time = &efi_set_time_boottime,
>>>    	.get_wakeup_time = (void *)&efi_unimplemented,
>>>    	.set_wakeup_time = (void *)&efi_unimplemented,
>>> +#ifdef CONFIG_RUNTIME_SET_VIRTUAL_ADDRESS_MAP
>>>    	.set_virtual_address_map = &efi_set_virtual_address_map,
>>> +#else
>>> +	.set_virtual_address_map = (void *)&efi_unimplemented,
>>
>> Depending on the ABI it is not save to use a function with another set
>> of parameters.
>>
>> Best regards
>>
>> Heinrich
>>
>>> +#endif
>>>    	.convert_pointer = (void *)&efi_invalid_parameter,
>>>    	.get_variable = efi_get_variable,
>>>    	.get_next_variable_name = efi_get_next_variable_name,
>>>
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>