Re: [PATCH v2 6/6] efi_loader: always initialize the secure boot state

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Thu, Aug 26, 2021 at 03:48:05PM +0200, Heinrich Schuchardt wrote:
> Even if we cannot read the variable store from disk we still need to
> initialize the secure boot state.
> 
> Don't continue to boot if the variable preseed is invalid as this indicates
> that the variable store has been tampered.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	no change
> ---
>  lib/efi_loader/efi_variable.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 80996d0f47..6d92229e2a 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -427,13 +427,17 @@ efi_status_t efi_init_variables(void)
>  	if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
>  		ret = efi_var_restore((struct efi_var_file *)
>  				      __efi_var_file_begin, true);
> -		if (ret != EFI_SUCCESS)
> +		if (ret != EFI_SUCCESS) {
>  			log_err("Invalid EFI variable seed\n");
> +			return ret;
> +		}
>  	}
> -
> -	ret = efi_var_from_file();
> +	ret = efi_init_secure_state();
>  	if (ret != EFI_SUCCESS)
>  		return ret;
>  
> -	return efi_init_secure_state();
> +	/* Don't stop booting if variable store is not available */
> +	efi_var_from_file();

I think we have to think about two different cases:
1) there is no "variable store" file available.
2) it does exists, but reading from it (efi_var_restore()) failed

For (2), we should return with an error as in the case of
CONFIG_EFI_VARIABLES_PRESEED.
Otherwise, the behavior is inconsistent.

- Takahiro Akashi

> +
> +	return EFI_SUCCESS;
>  }
> -- 
> 2.30.2
>