On Thu, Sep 17, 2026 at 06:26:37PM +0300, Ilias Apalodimas wrote:
> Hi Vincent,
>
> On Tue Sep 15, 2026 at 5:29 PM EEST, Vincent Stehlé wrote:
> > When the trial_counter_read() function reads its counter from the
> > TrialStateCtr UEFI Variable, it erroneously computes the return buffer
> > size to be the size of a pointer instead of a u16.
> >
> > As the UEFI Variable is always created by trial_counter_update() with
> > the right size before any read attempt, overflow cannot happen in
> > practice, but fix the size computation for correctness.
>
> The fix is correct, but the commit message if a bit off. The creation
> is not the reason the read works. It works because the current sizeof is
> always > of the new updated sizeof so the GetVariable calls has enough size
> to fix the output.
Hi Ilias,
Thank you for reviewing.
Is that not the other way around?
In the patch the call to GetVariable() was truncated but here are the
details:
status = efi_get_variable_int(
u"TrialStateCtr",
&efi_global_variable_guid,
NULL,
&var_size, // data_size
trial_state_ctr, // data
NULL);
We are telling GetVariable() to read as much as 4 to 8 bytes
(sizeof(trial_state_ctr), the size of a pointer, in var_size), into a 2
bytes buffer (the u16 pointed to by trial_state_ctr).
I think what saves us here is that we are sure the variable contains
only 2 bytes, which will never overflow.
Best regards,
Vincent.
>
>
> Cheers
> /Ilias
> >
> > Fixes: 7e9814cc6c1d ("FWU: Add boot time checks as highlighted by the FWU
> > specification")
> > Signed-off-by: Vincent Stehlé <[email protected]>
> > Cc: Sughosh Ganu <[email protected]>
> > Cc: Kory Maincent <[email protected]>
> > Cc: Tom Rini <[email protected]>
> > ---
> > Hi,
> >
> > This has been tested on the sandbox, with CONFIG_FWU_MULTI_BANK_UPDATE=y
> > & CONFIG_FWU_MDATA_V2=y, and with a modified fwu_boottime_checks()
> > function doing essentially the following sequence:
> >
> > efi_init_obj_list();
> > trial_counter_update();
> > trial_counter_read();
> > ---
> > lib/fwu_updates/fwu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> > index e9bb1b4fc65..24a8f1ce5b2 100644
> > --- a/lib/fwu_updates/fwu.c
> > +++ b/lib/fwu_updates/fwu.c
> > @@ -82,7 +82,7 @@ static int trial_counter_read(u16 *trial_state_ctr)
> > efi_status_t status;
> > efi_uintn_t var_size;
> >
> > - var_size = (efi_uintn_t)sizeof(trial_state_ctr);
> > + var_size = (efi_uintn_t)sizeof(*trial_state_ctr);
> > status = efi_get_variable_int(u"TrialStateCtr",
> > &efi_global_variable_guid,
> > NULL,
> >
> > ---
> > base-commit: a44f46af0aa17e48572e0d46c0d082886ee353c3
> > change-id: 20260914-fwu-24265a691f9a
> >
> > Best regards,
> > --
> > Vincent.
>
