On February 21, 2024 12:17:30 PM PST, ross.philip...@oracle.com wrote:
>On 2/15/24 1:01 AM, Ard Biesheuvel wrote:
>> On Wed, 14 Feb 2024 at 23:32, Ross Philipson <ross.philip...@oracle.com>
>> wrote:
>>>
>>> This support allows the DRTM launch to be initiated after an EFI stub
>>> launch of the Linux kernel is done. This is accomplished by providing
>>> a handler to jump to when a Secure Launch is in progress. This has to be
>>> called after the EFI stub does Exit Boot Services.
>>>
>>> Signed-off-by: Ross Philipson <ross.philip...@oracle.com>
>>> ---
>>> drivers/firmware/efi/libstub/x86-stub.c | 55 +++++++++++++++++++++++++
>>> 1 file changed, 55 insertions(+)
>>>
>>> diff --git a/drivers/firmware/efi/libstub/x86-stub.c
>>> b/drivers/firmware/efi/libstub/x86-stub.c
>>> index 0d510c9a06a4..4df2cf539194 100644
>>> --- a/drivers/firmware/efi/libstub/x86-stub.c
>>> +++ b/drivers/firmware/efi/libstub/x86-stub.c
>>> @@ -9,6 +9,7 @@
>>> #include <linux/efi.h>
>>> #include <linux/pci.h>
>>> #include <linux/stddef.h>
>>> +#include <linux/slr_table.h>
>>>
>>> #include <asm/efi.h>
>>> #include <asm/e820/types.h>
>>> @@ -810,6 +811,57 @@ static efi_status_t efi_decompress_kernel(unsigned
>>> long *kernel_entry)
>>> return EFI_SUCCESS;
>>> }
>>>
>>> +static void efi_secure_launch(struct boot_params *boot_params)
>>> +{
>>> + struct slr_entry_uefi_config *uefi_config;
>>> + struct slr_uefi_cfg_entry *uefi_entry;
>>> + struct slr_entry_dl_info *dlinfo;
>>> + efi_guid_t guid = SLR_TABLE_GUID;
>>> + struct slr_table *slrt;
>>> + u64 memmap_hi;
>>> + void *table;
>>> + u8 buf[64] = {0};
>>> +
>>
>> If you add a flex array to slr_entry_uefi_config as I suggested in
>> response to the other patch, we could simplify this substantially
>
>I feel like there is some reason why we did not use flex arrays. We were
>talking and we seem to remember we used to use them and someone asked us to
>remove them. We are still looking into it. But if we can go back to them, I
>will take all the changes you recommended here.
>
>Thanks
>Ross
>
>>
>> static struct slr_entry_uefi_config cfg = {
>> .hdr.tag = SLR_ENTRY_UEFI_CONFIG,
>> .hdr.size = sizeof(cfg),
>> .revision = SLR_UEFI_CONFIG_REVISION,
>> .nr_entries = 1,
>> .entries[0] = {
>> .pcr = 18,
>> .evt_info = "Measured UEFI memory map",
>> },
>> };
>>
>> cfg.entries[0].cfg = boot_params->efi_info.efi_memmap |
>> (u64)boot_params->efi_info.efi_memmap_hi << 32;
>> cfg.entries[0].size = boot_params->efi_info.efi_memmap_size;
>>
>>
>>
>>> + table = get_efi_config_table(guid);
>>> +
>>> + /*
>>> + * The presence of this table indicated a Secure Launch
>>> + * is being requested.
>>> + */
>>> + if (!table)
>>> + return;
>>> +
>>> + slrt = (struct slr_table *)table;
>>> +
>>> + if (slrt->magic != SLR_TABLE_MAGIC)
>>> + return;
>>> +
>>
>> slrt = (struct slr_table *)get_efi_config_table(guid);
>> if (!slrt || slrt->magic != SLR_TABLE_MAGIC)
>> return;
>>
>>> + /* Add config information to measure the UEFI memory map */
>>> + uefi_config = (struct slr_entry_uefi_config *)buf;
>>> + uefi_config->hdr.tag = SLR_ENTRY_UEFI_CONFIG;
>>> + uefi_config->hdr.size = sizeof(*uefi_config) + sizeof(*uefi_entry);
>>> + uefi_config->revision = SLR_UEFI_CONFIG_REVISION;
>>> + uefi_config->nr_entries = 1;
>>> + uefi_entry = (struct slr_uefi_cfg_entry *)(buf +
>>> sizeof(*uefi_config));
>>> + uefi_entry->pcr = 18;
>>> + uefi_entry->cfg = boot_params->efi_info.efi_memmap;
>>> + memmap_hi = boot_params->efi_info.efi_memmap_hi;
>>> + uefi_entry->cfg |= memmap_hi << 32;
>>> + uefi_entry->size = boot_params->efi_info.efi_memmap_size;
>>> + memcpy(&uefi_entry->evt_info[0], "Measured UEFI memory map",
>>> + strlen("Measured UEFI memory map"));
>>> +
>>
>> Drop all of this
>>
>>> + if (slr_add_entry(slrt, (struct slr_entry_hdr *)uefi_config))
>>
>> if (slr_add_entry(slrt, &uefi_config.hdr))
>>
>>
>>> + return;
>>> +
>>> + /* Jump through DL stub to initiate Secure Launch */
>>> + dlinfo = (struct slr_entry_dl_info *)
>>> + slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_DL_INFO);
>>> +
>>> + asm volatile ("jmp *%%rax"
>>> + : : "a" (dlinfo->dl_handler), "D"
>>> (&dlinfo->bl_context));
>>
>> Fix the prototype and just do
>>
>> dlinfo->dl_handler(&dlinfo->bl_context);
>> unreachable();
>>
>>
>> So in summary, this becomes
>>
>> static void efi_secure_launch(struct boot_params *boot_params)
>> {
>> static struct slr_entry_uefi_config cfg = {
>> .hdr.tag = SLR_ENTRY_UEFI_CONFIG,
>> .hdr.size = sizeof(cfg),
>> .revision = SLR_UEFI_CONFIG_REVISION,
>> .nr_entries = 1,
>> .entries[0] = {
>> .pcr = 18,
>> .evt_info = "Measured UEFI memory map",
>> },
>> };
>> struct slr_entry_dl_info *dlinfo;
>> efi_guid_t guid = SLR_TABLE_GUID;
>> struct slr_table *slrt;
>>
>> /*
>> * The presence of this table indicated a Secure Launch
>> * is being requested.
>> */
>> slrt = (struct slr_table *)get_efi_config_table(guid);
>> if (!slrt || slrt->magic != SLR_TABLE_MAGIC)
>> return;
>>
>> cfg.entries[0].cfg = boot_params->efi_info.efi_memmap |
>> (u64)boot_params->efi_info.efi_memmap_hi <<
>> 32;
>> cfg.entries[0].size = boot_params->efi_info.efi_memmap_size;
>>
>> if (slr_add_entry(slrt, &cfg.hdr))
>> return;
>>
>> /* Jump through DL stub to initiate Secure Launch */
>> dlinfo = (struct slr_entry_dl_info *)
>> slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_DL_INFO);
>>
>> dlinfo->dl_handler(&dlinfo->bl_context);
>>
>> unreachable();
>> }
>>
>>
>>> +}
>>> +
>>> static void __noreturn enter_kernel(unsigned long kernel_addr,
>>> struct boot_params *boot_params)
>>> {
>>> @@ -934,6 +986,9 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
>>> goto fail;
>>> }
>>>
>>> + /* If a Secure Launch is in progress, this never returns */
>>
>> if (IS_ENABLED(CONFIG_SECURE_LAUNCH))
>>
>>> + efi_secure_launch(boot_params);
>>> +
>>> /*
>>> * Call the SEV init code while still running with the firmware's
>>> * GDT/IDT, so #VC exceptions will be handled by EFI.
>>> --
>>> 2.39.3
>>>
>>
>
Linux kernel code doesn't use VLAs because of the limited stack size, and VLAs
or alloca() makes stack size tracking impossible. Although this technically
speaking runs in a different environment, it is easier to enforce the
constraint globally.
