| Message ID | 20240914160812.43632-1-heinrich.schuchardt@canonical.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | [v2,1/1] efi_leader: delete rng-seed if having EFI RNG protocol | expand |
Hi Heinrich, On Sat, Sep 14, 2024 at 06:08:12PM +0200, Heinrich Schuchardt wrote: > For measured be boot we must avoid any volatile values in the device-tree. > We already delete /chosen/kaslr-seed if we provide and EFI RNG protocol. > > Additionally remove /chosen/rng-seed provided by QEMU or U-Boot. > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > --- > v2: > put log_debug() in else branch > --- > include/efi_loader.h | 2 +- > lib/efi_loader/efi_dt_fixup.c | 16 +++++++++++----- > lib/efi_loader/efi_helper.c | 2 +- > 3 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index f84852e384f..511281e150e 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -567,7 +567,7 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, > /* 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); > +void efi_try_purge_rng_seed(void *fdt); > /* Called by bootefi to make console interface available */ > efi_status_t efi_console_register(void); > /* Called by efi_init_obj_list() to proble all block devices */ > diff --git a/lib/efi_loader/efi_dt_fixup.c b/lib/efi_loader/efi_dt_fixup.c > index 9d017804eea..b97758d1305 100644 > --- a/lib/efi_loader/efi_dt_fixup.c > +++ b/lib/efi_loader/efi_dt_fixup.c > @@ -41,7 +41,7 @@ static void efi_reserve_memory(u64 addr, u64 size, bool nomap) > } > > /** > - * efi_try_purge_kaslr_seed() - Remove unused kaslr-seed > + * efi_try_purge_rng_seed() - Remove unused kaslr-seed, rng-seed > * > * Kernel's EFI STUB only relies on EFI_RNG_PROTOCOL for randomization > * and completely ignores the kaslr-seed for its own randomness needs > @@ -51,8 +51,9 @@ static void efi_reserve_memory(u64 addr, u64 size, bool nomap) > * > * @fdt: Pointer to device tree > */ > -void efi_try_purge_kaslr_seed(void *fdt) > +void efi_try_purge_rng_seed(void *fdt) > { > + const char * const prop[] = {"kaslr-seed", "rng-seed"}; > const efi_guid_t efi_guid_rng_protocol = EFI_RNG_PROTOCOL_GUID; > struct efi_handler *handler; > efi_status_t ret; > @@ -67,9 +68,14 @@ void efi_try_purge_kaslr_seed(void *fdt) > 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"); > + for (const char * const *pos = prop; pos < &prop[ARRAY_SIZE(prop)]; I think for (int i = 0; i < ARRAY_SIZE(prop); i++) fdt_delprop(fdt, nodeoff, prop[i]); is a lot easier to read Other than that, the patch looks fine Thanks /Ilias > + ++pos) { > + err = fdt_delprop(fdt, nodeoff, *pos); > + if (err < 0 && err != -FDT_ERR_NOTFOUND) > + log_err("Error deleting %s\n", *pos); > + else > + log_debug("Deleted /chosen/%s\n", *pos); > + } > } > > /** > diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c > index 96f847652ec..a481eb4b7e3 100644 > --- a/lib/efi_loader/efi_helper.c > +++ b/lib/efi_loader/efi_helper.c > @@ -522,7 +522,7 @@ 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); > + efi_try_purge_rng_seed(fdt); > > if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) { > ret = efi_tcg2_measure_dtb(fdt); > -- > 2.45.2 >
On 9/17/24 08:38, Ilias Apalodimas wrote: > Hi Heinrich, > > On Sat, Sep 14, 2024 at 06:08:12PM +0200, Heinrich Schuchardt wrote: >> For measured be boot we must avoid any volatile values in the device-tree. >> We already delete /chosen/kaslr-seed if we provide and EFI RNG protocol. >> >> Additionally remove /chosen/rng-seed provided by QEMU or U-Boot. >> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >> --- >> v2: >> put log_debug() in else branch >> --- >> include/efi_loader.h | 2 +- >> lib/efi_loader/efi_dt_fixup.c | 16 +++++++++++----- >> lib/efi_loader/efi_helper.c | 2 +- >> 3 files changed, 13 insertions(+), 7 deletions(-) >> >> diff --git a/include/efi_loader.h b/include/efi_loader.h >> index f84852e384f..511281e150e 100644 >> --- a/include/efi_loader.h >> +++ b/include/efi_loader.h >> @@ -567,7 +567,7 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, >> /* 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); >> +void efi_try_purge_rng_seed(void *fdt); >> /* Called by bootefi to make console interface available */ >> efi_status_t efi_console_register(void); >> /* Called by efi_init_obj_list() to proble all block devices */ >> diff --git a/lib/efi_loader/efi_dt_fixup.c b/lib/efi_loader/efi_dt_fixup.c >> index 9d017804eea..b97758d1305 100644 >> --- a/lib/efi_loader/efi_dt_fixup.c >> +++ b/lib/efi_loader/efi_dt_fixup.c >> @@ -41,7 +41,7 @@ static void efi_reserve_memory(u64 addr, u64 size, bool nomap) >> } >> >> /** >> - * efi_try_purge_kaslr_seed() - Remove unused kaslr-seed >> + * efi_try_purge_rng_seed() - Remove unused kaslr-seed, rng-seed >> * >> * Kernel's EFI STUB only relies on EFI_RNG_PROTOCOL for randomization >> * and completely ignores the kaslr-seed for its own randomness needs >> @@ -51,8 +51,9 @@ static void efi_reserve_memory(u64 addr, u64 size, bool nomap) >> * >> * @fdt: Pointer to device tree >> */ >> -void efi_try_purge_kaslr_seed(void *fdt) >> +void efi_try_purge_rng_seed(void *fdt) >> { >> + const char * const prop[] = {"kaslr-seed", "rng-seed"}; >> const efi_guid_t efi_guid_rng_protocol = EFI_RNG_PROTOCOL_GUID; >> struct efi_handler *handler; >> efi_status_t ret; >> @@ -67,9 +68,14 @@ void efi_try_purge_kaslr_seed(void *fdt) >> 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"); >> + for (const char * const *pos = prop; pos < &prop[ARRAY_SIZE(prop)]; > > I think > for (int i = 0; i < ARRAY_SIZE(prop); i++) > fdt_delprop(fdt, nodeoff, prop[i]); > > is a lot easier to read On RISC-V using an index needs 10 bytes more of binary code. Easy to read and efficient are conflicting here. I will respin the patch. Best regards Heinrich > > Other than that, the patch looks fine > > Thanks > /Ilias > >> + ++pos) { >> + err = fdt_delprop(fdt, nodeoff, *pos); >> + if (err < 0 && err != -FDT_ERR_NOTFOUND) >> + log_err("Error deleting %s\n", *pos); >> + else >> + log_debug("Deleted /chosen/%s\n", *pos); >> + } >> } >> >> /** >> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c >> index 96f847652ec..a481eb4b7e3 100644 >> --- a/lib/efi_loader/efi_helper.c >> +++ b/lib/efi_loader/efi_helper.c >> @@ -522,7 +522,7 @@ 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); >> + efi_try_purge_rng_seed(fdt); >> >> if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) { >> ret = efi_tcg2_measure_dtb(fdt); >> -- >> 2.45.2 >>
diff --git a/include/efi_loader.h b/include/efi_loader.h index f84852e384f..511281e150e 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -567,7 +567,7 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, /* 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); +void efi_try_purge_rng_seed(void *fdt); /* Called by bootefi to make console interface available */ efi_status_t efi_console_register(void); /* Called by efi_init_obj_list() to proble all block devices */ diff --git a/lib/efi_loader/efi_dt_fixup.c b/lib/efi_loader/efi_dt_fixup.c index 9d017804eea..b97758d1305 100644 --- a/lib/efi_loader/efi_dt_fixup.c +++ b/lib/efi_loader/efi_dt_fixup.c @@ -41,7 +41,7 @@ static void efi_reserve_memory(u64 addr, u64 size, bool nomap) } /** - * efi_try_purge_kaslr_seed() - Remove unused kaslr-seed + * efi_try_purge_rng_seed() - Remove unused kaslr-seed, rng-seed * * Kernel's EFI STUB only relies on EFI_RNG_PROTOCOL for randomization * and completely ignores the kaslr-seed for its own randomness needs @@ -51,8 +51,9 @@ static void efi_reserve_memory(u64 addr, u64 size, bool nomap) * * @fdt: Pointer to device tree */ -void efi_try_purge_kaslr_seed(void *fdt) +void efi_try_purge_rng_seed(void *fdt) { + const char * const prop[] = {"kaslr-seed", "rng-seed"}; const efi_guid_t efi_guid_rng_protocol = EFI_RNG_PROTOCOL_GUID; struct efi_handler *handler; efi_status_t ret; @@ -67,9 +68,14 @@ void efi_try_purge_kaslr_seed(void *fdt) 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"); + for (const char * const *pos = prop; pos < &prop[ARRAY_SIZE(prop)]; + ++pos) { + err = fdt_delprop(fdt, nodeoff, *pos); + if (err < 0 && err != -FDT_ERR_NOTFOUND) + log_err("Error deleting %s\n", *pos); + else + log_debug("Deleted /chosen/%s\n", *pos); + } } /** diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c index 96f847652ec..a481eb4b7e3 100644 --- a/lib/efi_loader/efi_helper.c +++ b/lib/efi_loader/efi_helper.c @@ -522,7 +522,7 @@ 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); + efi_try_purge_rng_seed(fdt); if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) { ret = efi_tcg2_measure_dtb(fdt);
For measured be boot we must avoid any volatile values in the device-tree. We already delete /chosen/kaslr-seed if we provide and EFI RNG protocol. Additionally remove /chosen/rng-seed provided by QEMU or U-Boot. Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> --- v2: put log_debug() in else branch --- include/efi_loader.h | 2 +- lib/efi_loader/efi_dt_fixup.c | 16 +++++++++++----- lib/efi_loader/efi_helper.c | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-)