[PATCH v2] efi_loader: update SetVariable attribute check
Masahisa Kojima
masahisa.kojima at linaro.org
Wed Feb 15 10:38:30 CET 2023
Hi Heinrich,
On Wed, 15 Feb 2023 at 17:49, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 2/15/23 03:29, Masahisa Kojima wrote:
> > UEFI specification v2.10 says that
> > EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and
> > EFI_UNSUPPORTED should be returned in SetVariable variable service.
> > Current implementation returns EFI_INVALID_PARAMETER,
> > let's fix the return value.
> >
> > Together with above change, this commit also updates the SetVariable
> > attribute check to be aligned with the EDK2 reference implementation.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > ---
> > Changes in v2:
> > - fix coding style
> > - HR must be set with NV
> >
> > lib/efi_loader/efi_variable.c | 33 ++++++++++++++++++++++++++-------
> > 1 file changed, 26 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index 4c85cfa607..b12e79d658 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -230,9 +230,31 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
> > u64 time = 0;
> > enum efi_auth_var_type var_type;
> >
> > - if (!variable_name || !*variable_name || !vendor ||
> > - ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&
> > - !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)))
> > + if (!variable_name || !*variable_name || !vendor)
> > + return EFI_INVALID_PARAMETER;
> > +
> > + if (data_size && !data)
> > + return EFI_INVALID_PARAMETER;
> > +
> > + /* EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated */
> > + if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS ||
> > + (!(attributes & EFI_VARIABLE_MASK)))
>
> UEFI Spec 2.10, p. 215: "Setting a data variable with no access
> attributes causes it to be deleted."
>
> We don't want to make deletion unsupported.
Sorry, you are correct. I will remove the second condition.
Thanks,
Masahisa Kojima
>
> Best regards
>
> Heinrich
>
> > + return EFI_UNSUPPORTED;
> > +
> > + /* Make sure if runtime bit is set, boot service bit is set also */
> > + if ((attributes &
> > + (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)) ==
> > + EFI_VARIABLE_RUNTIME_ACCESS)
> > + return EFI_INVALID_PARAMETER;
> > +
> > + /* only EFI_VARIABLE_NON_VOLATILE attribute is invalid */
> > + if ((attributes & EFI_VARIABLE_MASK) == EFI_VARIABLE_NON_VOLATILE)
> > + return EFI_INVALID_PARAMETER;
> > +
> > + /* Make sure HR is set with NV */
> > + if ((attributes &
> > + (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_HARDWARE_ERROR_RECORD)) ==
> > + EFI_VARIABLE_HARDWARE_ERROR_RECORD)
> > return EFI_INVALID_PARAMETER;
> >
> > /* check if a variable exists */
> > @@ -281,8 +303,6 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
> >
> > /* authenticate a variable */
> > if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) {
> > - if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS)
> > - return EFI_INVALID_PARAMETER;
> > if (attributes &
> > EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
> > u32 env_attr;
> > @@ -300,8 +320,7 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
> > }
> > } else {
> > if (attributes &
> > - (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
> > - EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
> > + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
> > EFI_PRINT("Secure boot is not configured\n");
> > return EFI_INVALID_PARAMETER;
> > }
>
More information about the U-Boot
mailing list