Received-SPF: pass (google.com: domain of linux-crypto+bounces-2233-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 21 Feb 2024 12:37:25 -0800
From: "H. Peter Anvin" <hpa@zytor.com>
To: ross.philipson@oracle.com, Ard Biesheuvel <ardb@kernel.org>
CC: linux-kernel@vger.kernel.org, x86@kernel.org,
        linux-integrity@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-crypto@vger.kernel.org, kexec@lists.infradead.org,
        linux-efi@vger.kernel.org, dpsmith@apertussolutions.com,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, mjg59@srcf.ucam.org,
        James.Bottomley@hansenpartnership.com, peterhuewe@gmx.de,
        jarkko@kernel.org, jgg@ziepe.ca, luto@amacapital.net,
        nivedita@alum.mit.edu, herbert@gondor.apana.org.au,
        davem@davemloft.net, kanth.ghatraju@oracle.com,
        trenchboot-devel@googlegroups.com
Subject: =?US-ASCII?Q?Re=3A_=5BPATCH_v8_15/15=5D_x86=3A_EFI_stub_D?= =?US-ASCII?Q?RTM_launch_support_for_Secure_Launch?=
User-Agent: K-9 Mail for Android
In-Reply-To: <dc53f100-062b-47ae-abc8-5414ce8d041c@oracle.com>
References: <20240214221847.2066632-1-ross.philipson@oracle.com> <20240214221847.2066632-16-ross.philipson@oracle.com> <CAMj1kXF3k_c4Wn9GU+NC_+_aYfDpAzAUnfR=A4L_T+re1H3G=w@mail.gmail.com> <dc53f100-062b-47ae-abc8-5414ce8d041c@oracle.com>
Message-ID: <C98F883A-31D5-4F67-97FF-4AEFAADDDC74@zytor.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: quoted-printable

On February 21, 2024 12:17:30 PM PST, ross=2Ephilipson@oracle=2Ecom wrote:
>On 2/15/24 1:01 AM, Ard Biesheuvel wrote:
>> On Wed, 14 Feb 2024 at 23:32, Ross Philipson <ross=2Ephilipson@oracle=
=2Ecom> wrote:
>>>=20
>>> This support allows the DRTM launch to be initiated after an EFI stub
>>> launch of the Linux kernel is done=2E This is accomplished by providin=
g
>>> a handler to jump to when a Secure Launch is in progress=2E This has t=
o be
>>> called after the EFI stub does Exit Boot Services=2E
>>>=20
>>> Signed-off-by: Ross Philipson <ross=2Ephilipson@oracle=2Ecom>
>>> ---
>>>   drivers/firmware/efi/libstub/x86-stub=2Ec | 55 +++++++++++++++++++++=
++++
>>>   1 file changed, 55 insertions(+)
>>>=20
>>> diff --git a/drivers/firmware/efi/libstub/x86-stub=2Ec b/drivers/firmw=
are/efi/libstub/x86-stub=2Ec
>>> index 0d510c9a06a4=2E=2E4df2cf539194 100644
>>> --- a/drivers/firmware/efi/libstub/x86-stub=2Ec
>>> +++ b/drivers/firmware/efi/libstub/x86-stub=2Ec
>>> @@ -9,6 +9,7 @@
>>>   #include <linux/efi=2Eh>
>>>   #include <linux/pci=2Eh>
>>>   #include <linux/stddef=2Eh>
>>> +#include <linux/slr_table=2Eh>
>>>=20
>>>   #include <asm/efi=2Eh>
>>>   #include <asm/e820/types=2Eh>
>>> @@ -810,6 +811,57 @@ static efi_status_t efi_decompress_kernel(unsigne=
d long *kernel_entry)
>>>          return EFI_SUCCESS;
>>>   }
>>>=20
>>> +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 =3D SLR_TABLE_GUID;
>>> +       struct slr_table *slrt;
>>> +       u64 memmap_hi;
>>> +       void *table;
>>> +       u8 buf[64] =3D {0};
>>> +
>>=20
>> 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=2E We wer=
e talking and we seem to remember we used to use them and someone asked us =
to remove them=2E We are still looking into it=2E But if we can go back to =
them, I will take all the changes you recommended here=2E
>
>Thanks
>Ross
>
>>=20
>> static struct slr_entry_uefi_config cfg =3D {
>>          =2Ehdr=2Etag        =3D SLR_ENTRY_UEFI_CONFIG,
>>          =2Ehdr=2Esize       =3D sizeof(cfg),
>>          =2Erevision       =3D SLR_UEFI_CONFIG_REVISION,
>>          =2Enr_entries     =3D 1,
>>          =2Eentries[0]     =3D {
>>                  =2Epcr    =3D 18,
>>                  =2Eevt_info =3D "Measured UEFI memory map",
>>          },
>> };
>>=20
>> cfg=2Eentries[0]=2Ecfg  =3D boot_params->efi_info=2Eefi_memmap |
>>                        (u64)boot_params->efi_info=2Eefi_memmap_hi << 32=
;
>> cfg=2Eentries[0]=2Esize =3D boot_params->efi_info=2Eefi_memmap_size;
>>=20
>>=20
>>=20
>>> +       table =3D get_efi_config_table(guid);
>>> +
>>> +       /*
>>> +        * The presence of this table indicated a Secure Launch
>>> +        * is being requested=2E
>>> +        */
>>> +       if (!table)
>>> +               return;
>>> +
>>> +       slrt =3D (struct slr_table *)table;
>>> +
>>> +       if (slrt->magic !=3D SLR_TABLE_MAGIC)
>>> +               return;
>>> +
>>=20
>> slrt =3D (struct slr_table *)get_efi_config_table(guid);
>> if (!slrt || slrt->magic !=3D SLR_TABLE_MAGIC)
>>          return;
>>=20
>>> +       /* Add config information to measure the UEFI memory map */
>>> +       uefi_config =3D (struct slr_entry_uefi_config *)buf;
>>> +       uefi_config->hdr=2Etag =3D SLR_ENTRY_UEFI_CONFIG;
>>> +       uefi_config->hdr=2Esize =3D sizeof(*uefi_config) + sizeof(*uef=
i_entry);
>>> +       uefi_config->revision =3D SLR_UEFI_CONFIG_REVISION;
>>> +       uefi_config->nr_entries =3D 1;
>>> +       uefi_entry =3D (struct slr_uefi_cfg_entry *)(buf + sizeof(*uef=
i_config));
>>> +       uefi_entry->pcr =3D 18;
>>> +       uefi_entry->cfg =3D boot_params->efi_info=2Eefi_memmap;
>>> +       memmap_hi =3D boot_params->efi_info=2Eefi_memmap_hi;
>>> +       uefi_entry->cfg |=3D memmap_hi << 32;
>>> +       uefi_entry->size =3D boot_params->efi_info=2Eefi_memmap_size;
>>> +       memcpy(&uefi_entry->evt_info[0], "Measured UEFI memory map",
>>> +               strlen("Measured UEFI memory map"));
>>> +
>>=20
>> Drop all of this
>>=20
>>> +       if (slr_add_entry(slrt, (struct slr_entry_hdr *)uefi_config))
>>=20
>> if (slr_add_entry(slrt, &uefi_config=2Ehdr))
>>=20
>>=20
>>> +               return;
>>> +
>>> +       /* Jump through DL stub to initiate Secure Launch */
>>> +       dlinfo =3D (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_c=
ontext));
>>=20
>> Fix the prototype and just do
>>=20
>> dlinfo->dl_handler(&dlinfo->bl_context);
>> unreachable();
>>=20
>>=20
>> So in summary, this becomes
>>=20
>> static void efi_secure_launch(struct boot_params *boot_params)
>> {
>>          static struct slr_entry_uefi_config cfg =3D {
>>                  =2Ehdr=2Etag        =3D SLR_ENTRY_UEFI_CONFIG,
>>                  =2Ehdr=2Esize       =3D sizeof(cfg),
>>                  =2Erevision       =3D SLR_UEFI_CONFIG_REVISION,
>>                  =2Enr_entries     =3D 1,
>>                  =2Eentries[0]     =3D {
>>                          =2Epcr    =3D 18,
>>                          =2Eevt_info =3D "Measured UEFI memory map",
>>                  },
>>          };
>>          struct slr_entry_dl_info *dlinfo;
>>          efi_guid_t guid =3D SLR_TABLE_GUID;
>>          struct slr_table *slrt;
>>=20
>>          /*
>>           * The presence of this table indicated a Secure Launch
>>           * is being requested=2E
>>           */
>>          slrt =3D (struct slr_table *)get_efi_config_table(guid);
>>          if (!slrt || slrt->magic !=3D SLR_TABLE_MAGIC)
>>                  return;
>>=20
>>          cfg=2Eentries[0]=2Ecfg  =3D boot_params->efi_info=2Eefi_memmap=
 |
>>                                (u64)boot_params->efi_info=2Eefi_memmap_=
hi << 32;
>>          cfg=2Eentries[0]=2Esize =3D boot_params->efi_info=2Eefi_memmap=
_size;
>>=20
>>          if (slr_add_entry(slrt, &cfg=2Ehdr))
>>                  return;
>>=20
>>          /* Jump through DL stub to initiate Secure Launch */
>>          dlinfo =3D (struct slr_entry_dl_info *)
>>                   slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_DL_INFO);
>>=20
>>          dlinfo->dl_handler(&dlinfo->bl_context);
>>=20
>>          unreachable();
>> }
>>=20
>>=20
>>> +}
>>> +
>>>   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;
>>>          }
>>>=20
>>> +       /* If a Secure Launch is in progress, this never returns */
>>=20
>> if (IS_ENABLED(CONFIG_SECURE_LAUNCH))
>>=20
>>> +       efi_secure_launch(boot_params);
>>> +
>>>          /*
>>>           * Call the SEV init code while still running with the firmwa=
re's
>>>           * GDT/IDT, so #VC exceptions will be handled by EFI=2E
>>> --
>>> 2=2E39=2E3
>>>=20
>>=20
>

Linux kernel code doesn't use VLAs because of the limited stack size, and =
VLAs or alloca() makes stack size tracking impossible=2E Although this tech=
nically speaking runs in a different environment, it is easier to enforce t=
he constraint globally=2E