[PATCH] efi_loader: factor out the common code from efi_transfer_secure_state()

[Heinrich Schuchardt]

On 4/21/20 2:39 AM, AKASHI Takahiro wrote:
> efi_set_secure_stat() provides the common code for each stat transition
> caused by efi_transfer_secure_state().
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  lib/efi_loader/efi_variable.c | 194 +++++++++++-----------------------
>  1 file changed, 64 insertions(+), 130 deletions(-)
>
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 0c6d1deb58eb..c275684278a1 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -176,6 +176,59 @@ static efi_status_t efi_set_variable_internal(u16 *variable_name,
>  					      const void *data,
>  					      bool ro_check);
>
> +/**
> + * efi_set_secure_state - modify secure boot state variables
> + * @sec_boot:		value of SecureBoot
> + * @setup_mode:		value of SetupMode
> + * @audit_mode:		value of AuditMode
> + * @deployed_mode:	value of DeployedMode
> + *
> + * Modify secure boot stat-related variables as indicated.
> + *
> + * Return:	EFI_SUCCESS on success, status code (negative) on error

According to the UEFI spec:

EFI_STATUS Type UINTN

So the return value cannot be negative. I will adjust the text when merging.

Otherwise:

Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>

> + */
> +static efi_status_t efi_set_secure_state(int sec_boot, int setup_mode,
> +					 int audit_mode, int deployed_mode)
> +{
> +	u32 attributes;
> +	efi_status_t ret;
> +
> +	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +		     EFI_VARIABLE_RUNTIME_ACCESS |
> +		     READ_ONLY;
> +	ret = efi_set_variable_internal(L"SecureBoot",
> +					&efi_global_variable_guid,
> +					attributes,
> +					sizeof(sec_boot), &sec_boot,
> +					false);
> +	if (ret != EFI_SUCCESS)
> +		goto err;
> +
> +	ret = efi_set_variable_internal(L"SetupMode",
> +					&efi_global_variable_guid,
> +					attributes,
> +					sizeof(setup_mode), &setup_mode,
> +					false);
> +	if (ret != EFI_SUCCESS)
> +		goto err;
> +
> +	ret = efi_set_variable_internal(L"AuditMode",
> +					&efi_global_variable_guid,
> +					attributes,
> +					sizeof(audit_mode), &audit_mode,
> +					false);
> +	if (ret != EFI_SUCCESS)
> +		goto err;
> +
> +	ret = efi_set_variable_internal(L"DeployedMode",
> +					&efi_global_variable_guid,
> +					attributes,
> +					sizeof(deployed_mode), &deployed_mode,
> +					false);
> +err:
> +	return ret;
> +}
> +
>  /**
>   * efi_transfer_secure_state - handle a secure boot state transition
>   * @mode:	new state
> @@ -188,157 +241,38 @@ static efi_status_t efi_set_variable_internal(u16 *variable_name,
>   */
>  static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode)
>  {
> -	u32 attributes;
> -	u8 val;
>  	efi_status_t ret;
>
> -	debug("Secure state from %d to %d\n", efi_secure_mode, mode);
> +	debug("Switching secure state from %d to %d\n", efi_secure_mode, mode);
>
> -	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -		     EFI_VARIABLE_RUNTIME_ACCESS;
>  	if (mode == EFI_MODE_DEPLOYED) {
> -		val = 1;
> -		ret = efi_set_variable_internal(L"SecureBoot",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 0;
> -		ret = efi_set_variable_internal(L"SetupMode",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 0;
> -		ret = efi_set_variable_internal(L"AuditMode",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 1;
> -		ret = efi_set_variable_internal(L"DeployedMode",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> +		ret = efi_set_secure_state(1, 0, 0, 1);
>  		if (ret != EFI_SUCCESS)
>  			goto err;
>
>  		efi_secure_boot = true;
>  	} else if (mode == EFI_MODE_AUDIT) {
> -		ret = efi_set_variable_internal(L"PK",
> -						&efi_global_variable_guid,
> -						attributes,
> -						0, NULL,
> -						false);
> +		ret = efi_set_variable_internal(
> +					L"PK", &efi_global_variable_guid,
> +					EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +					EFI_VARIABLE_RUNTIME_ACCESS,
> +					0, NULL, false);
>  		if (ret != EFI_SUCCESS)
>  			goto err;
> -		val = 0;
> -		ret = efi_set_variable_internal(L"SecureBoot",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 1;
> -		ret = efi_set_variable_internal(L"SetupMode",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 1;
> -		ret = efi_set_variable_internal(L"AuditMode",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 0;
> -		ret = efi_set_variable_internal(L"DeployedMode",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> +
> +		ret = efi_set_secure_state(0, 1, 1, 0);
>  		if (ret != EFI_SUCCESS)
>  			goto err;
>
>  		efi_secure_boot = true;
>  	} else if (mode == EFI_MODE_USER) {
> -		val = 1;
> -		ret = efi_set_variable_internal(L"SecureBoot",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 0;
> -		ret = efi_set_variable_internal(L"SetupMode",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 0;
> -		ret = efi_set_variable_internal(L"AuditMode",
> -						&efi_global_variable_guid,
> -						attributes,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 0;
> -		ret = efi_set_variable_internal(L"DeployedMode",
> -						&efi_global_variable_guid,
> -						attributes,
> -						sizeof(val), &val,
> -						false);
> +		ret = efi_set_secure_state(1, 0, 0, 0);
>  		if (ret != EFI_SUCCESS)
>  			goto err;
>
>  		efi_secure_boot = true;
>  	} else if (mode == EFI_MODE_SETUP) {
> -		val = 0;
> -		ret = efi_set_variable_internal(L"SecureBoot",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 1;
> -		ret = efi_set_variable_internal(L"SetupMode",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 0;
> -		ret = efi_set_variable_internal(L"AuditMode",
> -						&efi_global_variable_guid,
> -						attributes,
> -						sizeof(val), &val,
> -						false);
> -		if (ret != EFI_SUCCESS)
> -			goto err;
> -		val = 0;
> -		ret = efi_set_variable_internal(L"DeployedMode",
> -						&efi_global_variable_guid,
> -						attributes | READ_ONLY,
> -						sizeof(val), &val,
> -						false);
> +		ret = efi_set_secure_state(0, 1, 0, 0);
>  		if (ret != EFI_SUCCESS)
>  			goto err;
>  	} else {
>