Am 2. Januar 2022 21:50:35 MEZ schrieb Ilias Apalodimas
<ilias.apalodi...@linaro.org>:
>Hi Heinrich,
>
>> > > > >
>
>[...]
>
>> > > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> > > > > index d77d3b6e943d..57f13ce701ec 100644
>> > > > > --- a/cmd/bootefi.c
>> > > > > +++ b/cmd/bootefi.c
>> > > > > @@ -310,6 +310,8 @@ efi_status_t efi_install_fdt(void *fdt)
>> > > > > /* Create memory reservations as indicated by the device tree */
>> > > > > efi_carve_out_dt_rsv(fdt);
>> > > > >
>> > > > > + efi_try_purge_kaslr_seed(fdt);
>>
>> This function should only be invoked for CONFIG_EFI_TCG2_PROTOCOL=y.
>
>Why? As we discussed the kernel ignores the kaslr-seed for the
>physical randomization. The only reason we would like to keep it is
>for the randomization of the virtual address. But if the EFI
>RNG protocol is installed the EFI stub is already doing the right thing.
>So I really think purging it if EFI RNG is installed is the best option
>here (regardless of TPM measurements)
>
The only reason to delete kaslr-seed is that it conflicts with measured boot.
If an OS prefers the RNG protocol over kaslr-seed is the decision of the OS and
nothing U-Boot has to care about.
You will have to delete kaslr-seed no matter if you have a RNG protocol or not
if and only if you want to use measured boot.
Best regards
Heinrich
>> > > > > +
>> > > > > /* Install device tree as UEFI table */
>> > > > > ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
>> > > > > if (ret != EFI_SUCCESS) {
>> > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h
>> > > > > index 9dd6c2033634..1fe003db69e0 100644
>> > > > > --- a/include/efi_loader.h
>> > > > > +++ b/include/efi_loader.h
>> > > > > @@ -519,6 +519,8 @@ efi_status_t EFIAPI
>> > > > > efi_convert_pointer(efi_uintn_t debug_disposition,
>> > > > > void **address);
>> > > > > /* Carve out DT reserved memory ranges */
>> > > > > void efi_carve_out_dt_rsv(void *fdt);
>> > > > > +/* Purge unused kaslr-seed */
>> > > > > +void efi_try_purge_kaslr_seed(void *fdt);
>> > > > > /* Called by bootefi to make console interface available */
>> > > > > efi_status_t efi_console_register(void);
>> > > > > /* Called by bootefi to make all disk storage accessible as EFI
>> > > > > objects */
>> > > > > diff --git a/lib/efi_loader/efi_dt_fixup.c
>> > > > > b/lib/efi_loader/efi_dt_fixup.c
>> > > > > index b6fe5d2e5a34..d3923e5dba1b 100644
>> > > > > --- a/lib/efi_loader/efi_dt_fixup.c
>> > > > > +++ b/lib/efi_loader/efi_dt_fixup.c
>> > > > > @@ -8,6 +8,7 @@
>> > > > > #include <common.h>
>> > > > > #include <efi_dt_fixup.h>
>> > > > > #include <efi_loader.h>
>> > > > > +#include <efi_rng.h>
>> > > > > #include <fdtdec.h>
>> > > > > #include <mapmem.h>
>> > > > >
>> > > > > @@ -40,6 +41,38 @@ static void efi_reserve_memory(u64 addr, u64
>> > > > > size, bool nomap)
>> > > > > addr, size);
>> > > > > }
>> > > > >
>> > > > > +/**
>> > > > > + * efi_try_purge_kaslr_seed() - Remove unused kaslr-seed
>> > > > > + *
>> > > > > + * Kernel's EFI STUB only relies on EFI_RNG_PROTOCOL for
>> > > > > randomization
>> > > > > + * and completely ignores the kaslr-seed for its own randomness
>> > > > > needs
>> > > > > + * (i.e the randomization of the physical placement of the kernel).
>> > > > > + * Weed it out from the DTB we hand over, which would mess up our
>> > > > > DTB
>> > > > > + * TPM measurements as well.
>> > > > > + *
>> > > > > + * @fdt: Pointer to device tree
>> > > > > + */
>> > > > > +void efi_try_purge_kaslr_seed(void *fdt)
>> > > > > +{
>> > > > > + const efi_guid_t efi_guid_rng_protocol = EFI_RNG_PROTOCOL_GUID;
>>
>> There is not need to check if the RNG protocol is installed. If
>> CONFIG_EFI_TCG2_PROTOCOL=y, you should unconditionally remove
>> 'kaslr-seed' as it is incompatible with measured boot.
>
>That's not entirely correct. Right now having the kaslr-seed hurts no one,
>since we don't measure the DTB to begin with. What I intend to do is
>expose the RNG hardware of the TPM and use that if the hardware doesn't
>provide one already. This obviously means the kaslr-seed will be removed
>because the RNG protocol will always be installed with the current patch.
>
>I really don't see a connection between a *compile* time option which
>might not even have any effect if a TPM is not present, with an entry in
>the /chosen node. IMHO we should merge this patch since it improves
>existing use cases. I'll work on the rest and send patches soon.
>
>Cheers
>/Ilias
>
>
>>
>> Best regards
>>
>> Heinrich
>>
>> > > > > + struct efi_handler *handler;
>> > > > > + efi_status_t ret;
>> > > > > + int nodeoff = 0;
>> > > > > + int err = 0;
>> > > > > +
>> > > > > + ret = efi_search_protocol(efi_root, &efi_guid_rng_protocol,
>> > > > > &handler);
>> > > > > + if (ret != EFI_SUCCESS)
>> > > > > + return;
>> > > > > +
>> > > > > + nodeoff = fdt_path_offset(fdt, "/chosen");
>> > > > > + if (nodeoff < 0)
>> > > > > + return;
>> > > > > +
>> > > > > + err = fdt_delprop(fdt, nodeoff, "kaslr-seed");
>> > > > > + if (err < 0 && err != -FDT_ERR_NOTFOUND)
>> > > > > + log_err("Error deleting kaslr-seed\n");
>> > > > > +}
>> > > > > +
>> > > > > /**
>> > > > > * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges
>> > > > > *
>> > > > > --
>> > > > > 2.30.2
>> > > > >
>> > > > >
>> > >
