Re: [PATCH v4 1/2] efi: Add a function to check if Secure Boot mode is enabled

[Gerald Elder-Vass <gerald.elder-vass@xxxxxxxxx>]

> On Mon, Sep 08, 2025 at 11:41:55AM +0200, Jan Beulich wrote:
> > On 08.09.2025 11:35, Gerald Elder-Vass wrote:
> > >>> +          size == 1 && data == 0) )
> > >>
> > >> ... any reason it's literal 1 here?
> > > 
> > > The size variable is also used as output from GetVariable and we should
> > > verify that the size of the returned data is as expected, it is simply one
> > > byte so probably not worth defining any macros to make it clearer
> > 
> > I don't understand this reply. Why would the initializer of the variable
> > use one thing (sizeof()) and the checking of the variable another (literal
> > 1)? Even consistently using 1 would already be better imo; consistently
> > using sizeof() is what I think would be best.
>
 > 'size' as input value is the allocated size of the data parameter, so
> makes sense to be sizeof(data). IOW, 'size' as the input value comes
> from the size of the 'data' variable, while the output value check comes
> from UEFI spec. While the size of the 'data' variable should match the
>spec, IMO changing its type (to a wider one) should not break the
>behavior here.

 The UEFI spec defines the "SecureBoot" global variable as an 8-bit unsigned

integer, in the event the size of the data used for output was not large enough to

contain the output then it would return an EFI_BUFFER_TOO_SMALL status

(which the function would then interpret as "play it safe and assume enabled").

The "SecureBoot" variable is defined:

https://uefi.org/specs/UEFI/2.11/03_Boot_Manager.html#globally-defined-variables

So I believe we are correct in using uint8_t here

Gerald Elder-Vass

Senior Software Engineer

XenServer

Cambridge, UK

On Mon, Sep 8, 2025 at 10:53 AM Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:

On Mon, Sep 08, 2025 at 11:41:55AM +0200, Jan Beulich wrote:
> On 08.09.2025 11:35, Gerald Elder-Vass wrote:
> >>> +          size == 1 && data == 0) )
> >>
> >> ... any reason it's literal 1 here?
> >
> > The size variable is also used as output from GetVariable and we should
> > verify that the size of the returned data is as expected, it is simply one
> > byte so probably not worth defining any macros to make it clearer
>
> I don't understand this reply. Why would the initializer of the variable
> use one thing (sizeof()) and the checking of the variable another (literal
> 1)? Even consistently using 1 would already be better imo; consistently
> using sizeof() is what I think would be best.

'size' as input value is the allocated size of the data parameter, so
makes sense to be sizeof(data). IOW, 'size' as the input value comes
from the size of the 'data' variable, while the output value check comes
from UEFI spec. While the size of the 'data' variable should match the
spec, IMO changing its type (to a wider one) should not break the
behavior here.

--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab