Messages in this thread | | | Date | Thu, 3 Mar 2016 09:03:25 +0100 | Subject | Re: [PATCH v2 1/2] arm64/efi: report unexpected errors when determining Secure Boot status | From | Ard Biesheuvel <> |
| |
Hi Linn,
Apologies for the delay in reviewing this.
On 26 February 2016 at 01:18, Linn Crosetto <linn@hpe.com> wrote: > Certain code in the boot path may require the ability to determine whether > UEFI Secure Boot is definitely enabled, for example printing status to the > console. Other code may need to know when UEFI Secure Boot is definitely > disabled, for example restricting use of kernel parameters. > > If an unexpected error is returned from GetVariable when querying the > status of UEFI Secure Boot, return an error to the caller. This allows the > caller to determine the definite state, and to take appropriate action if > an expected error is returned. > > Signed-off-by: Linn Crosetto <linn@hpe.com> > --- > New patch in v2 based on feedback from v1: > > - Maintain existing behavior to allow 'dtb=' parameter only when UEFI > Secure Boot is disabled and not in an unknown state. (Mark Rutland) > > drivers/firmware/efi/libstub/arm-stub.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c > index 3397902..b1bb133 100644 > --- a/drivers/firmware/efi/libstub/arm-stub.c > +++ b/drivers/firmware/efi/libstub/arm-stub.c > @@ -18,7 +18,7 @@ > > #include "efistub.h" > > -static int efi_secureboot_enabled(efi_system_table_t *sys_table_arg) > +static int efi_get_secureboot(efi_system_table_t *sys_table_arg) > { > static efi_guid_t const var_guid = EFI_GLOBAL_VARIABLE_GUID; > static efi_char16_t const var_name[] = { > @@ -37,8 +37,12 @@ static int efi_secureboot_enabled(efi_system_table_t *sys_table_arg) > return val; > case EFI_NOT_FOUND: > return 0; > + case EFI_DEVICE_ERROR: > + return -EIO; > + case EFI_SECURITY_VIOLATION: > + return -EACCES; > default: > - return 1; > + return -EINVAL; > } > } > > @@ -183,6 +187,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table, > efi_guid_t loaded_image_proto = LOADED_IMAGE_PROTOCOL_GUID; > unsigned long reserve_addr = 0; > unsigned long reserve_size = 0; > + int secure_boot = 0; > > /* Check if we were booted by the EFI firmware */ > if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) > @@ -231,13 +236,15 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table, > if (status != EFI_SUCCESS) > pr_efi_err(sys_table, "Failed to parse EFI cmdline options\n"); > > + secure_boot = efi_get_secureboot(sys_table); > + if (secure_boot > 0) > + pr_efi(sys_table, "UEFI Secure Boot is enabled.\n"); > + > /* > * Unauthenticated device tree data is a security hazard, so > * ignore 'dtb=' unless UEFI Secure Boot is disabled. > */ > - if (efi_secureboot_enabled(sys_table)) { > - pr_efi(sys_table, "UEFI Secure Boot is enabled.\n"); > - } else { > + if (secure_boot == 0) {
There is a slight difference in behavior here: if we can't determine whether secure boot is enabled, we no longer print anything, but silently ignore the dtb= parameter.
Perhaps it is better to print 'could not determine secure boot status, assuming enabled' or something like that?
> status = handle_cmdline_files(sys_table, image, cmdline_ptr, > "dtb=", > ~0UL, &fdt_addr, &fdt_size); > -- > 2.1.4 >
| |