Re: [PATCH v3 4/4] efi: Do not use command line if secure boot is enabled.

[Trammell Hudson <hudson@trmm.net>]

On Monday, September 14, 2020 6:24 AM, Roger Pau Monné <roger.pau@citrix.com> wrote:
> On Mon, Sep 07, 2020 at 03:00:27PM -0400, Trammell Hudson wrote:
> [...]
> > -   static const __initconst EFI_GUID global_guid = EFI_GLOBAL_VARIABLE;
> > -   uint8_t secboot, setupmode;
> > -   UINTN secboot_size = sizeof(secboot);
> > -   UINTN setupmode_size = sizeof(setupmode);
> > -
> > -   if ( efi_rs->GetVariable(L"SecureBoot", (EFI_GUID *)&global_guid, NULL, &secboot_size, &secboot) != EFI_SUCCESS )
>
> I'm slightly worried about the dropping of the const here, and the
> fact that the variable is placed in initconst section. Isn't it
> dangerous that the EFI services will try to write to it?

The EFI services do not try to write to it; the API doesn't
even bother with const-correctness.  The prototype has IN
and OUT, but they are not used for constness:

typedef EFI_STATUS(EFIAPI * EFI_GET_VARIABLE) (
IN CHAR16 *VariableName,
IN EFI_GUID *VendorGuid,
OUT UINT32 *Attributes,
OPTIONAL IN OUT UINTN *DataSize,
OUT VOID *Data OPTIONAL)

(So the VariableName string is also silently being turned
into a non-const pointer as well, which is just ugh)

> [...]
> > -   return secboot == 1 && setupmode == 0;
>
> I would print a message if secboot is > 1, since those should be
> reserved.

Ok.  Addressed in v4, coming soon.

--
Trammell