Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1341915

Re: [PATCH] arm64/efi: check SetupMode when determining Secure Boot status

Show all headers | View raw

On Tue, Feb 23, 2016 at 05:25:09PM -0700, Linn Crosetto wrote:
> According to the UEFI specification, the platform is operating in secure
> boot mode if SetupMode is 0 and SecureBoot is 1, and cannot operate in
> secure boot mode if SetupMode is set to 1.

I see the above is from the third-last paragraph of section 3.3 Globally
Defined Variables, (in 2.5 Errata A).

For the commit message, it might be good to split the quote from the
rest of the message (e.g. by putting it in a separate indented
paragraph), to make it clear which part is from the spec.

> Check the value of SetupMode when determining the state of Secure
> Boot.

It sounds like we should be doing this. I have a couple of comments
below.

> Signed-off-by: Linn Crosetto <linn@hpe.com>
> ---
>  drivers/firmware/efi/libstub/arm-stub.c | 34 +++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 3397902..7ef2e20 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -20,26 +20,36 @@
>  
>  static int efi_secureboot_enabled(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[] = {
> +	static efi_char16_t const sb_var_name[] = {
>  		'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0 };
> +	static efi_char16_t const sm_var_name[] = {
> +		'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0 };
>  
> +	efi_guid_t var_guid = EFI_GLOBAL_VARIABLE_GUID;
>  	efi_get_variable_t *f_getvar = sys_table_arg->runtime->get_variable;
> -	unsigned long size = sizeof(u8);
> -	efi_status_t status;
>  	u8 val;
> +	unsigned long size = sizeof(val);

I think this variable could have stayed as it was, it's logically an
unrelated change. Otherwise, point out the cleanup in the commit
message.

> +	efi_status_t status;
>  
> -	status = f_getvar((efi_char16_t *)var_name, (efi_guid_t *)&var_guid,
> +	status = f_getvar((efi_char16_t *)sb_var_name, (efi_guid_t *)&var_guid,
>  			  NULL, &size, &val);
>  
> -	switch (status) {
> -	case EFI_SUCCESS:
> -		return val;
> -	case EFI_NOT_FOUND:
> +	if (status != EFI_SUCCESS)
>  		return 0;
> -	default:
> -		return 1;
> -	}

That isn't quite the same as the existing behaviour. Previously for any
return value other than EFI_SUCCESS, we would fail-safe and assume
secure boot was enabled, whereas now we'll assume it is not.

I think we should retain the existing behaviour.

Mark.

> +
> +	if (val == 0)
> +		return 0;
> +
> +	status = f_getvar((efi_char16_t *)sm_var_name, (efi_guid_t *)&var_guid,
> +			  NULL, &size, &val);
> +
> +	if (status != EFI_SUCCESS)
> +		return 0;
> +
> +	if (val == 1)
> +		return 0;
> +
> +	return 1;
>  }
>  
>  efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,
> -- 
> 2.1.4
> 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] arm64/efi: check SetupMode when determining Secure Boot status Linn Crosetto <linn@hpe.com> - 2016-02-24 01:30 +0100
  Re: [PATCH] arm64/efi: check SetupMode when determining Secure Boot  status Mark Rutland <mark.rutland@arm.com> - 2016-02-24 12:30 +0100
  [PATCH v2 2/2] arm64/efi: check SetupMode when determining Secure Boot status Linn Crosetto <linn@hpe.com> - 2016-02-26 01:20 +0100
  [PATCH v2 0/2] arm64/efi: query Secure Boot status according to UEFI spec Linn Crosetto <linn@hpe.com> - 2016-02-26 01:20 +0100
    [PATCH v2 1/2] arm64/efi: report unexpected errors when determining Secure Boot status Linn Crosetto <linn@hpe.com> - 2016-02-26 01:20 +0100
      Re: [PATCH v2 1/2] arm64/efi: report unexpected errors when  determining Secure Boot status Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-03-03 09:10 +0100
    Re: [PATCH v2 0/2] arm64/efi: query Secure Boot status according to  UEFI spec Matt Fleming <matt@codeblueprint.co.uk> - 2016-03-02 14:40 +0100
    [PATCH v3 2/2] arm64/efi: check SetupMode when determining Secure Boot status Linn Crosetto <linn@hpe.com> - 2016-03-03 22:50 +0100
      Re: [PATCH v3 2/2] arm64/efi: check SetupMode when determining Secure  Boot status Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-03-04 09:10 +0100
    [PATCH v3 0/2] arm64/efi: query Secure Boot status according to UEFI spec Linn Crosetto <linn@hpe.com> - 2016-03-03 22:50 +0100
      [PATCH v3 1/2] arm64/efi: report unexpected errors when determining Secure Boot status Linn Crosetto <linn@hpe.com> - 2016-03-03 22:50 +0100
        Re: [PATCH v3 1/2] arm64/efi: report unexpected errors when  determining Secure Boot status Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-03-04 09:00 +0100

csiph-web