[PATCH v4 1/5] efi_loader: store firmware version into FmpState variable

[Ilias Apalodimas]

Kojima-san
Apologies for the late review comments but I have some concerns about
the design and storing the info as EFI variables.  If the backing
storage is a file we can't trust any of that information since anyone
can tamper with the file, although the variables are defined as RO.

The new struct defines
struct fmp_state {
       u32 fw_version;
       u32 lowest_supported_version;
       u32 last_attempt_version;
       u32 last_attempt_status;
}

I think we should change the approach a bit.
fw_version -> this should go into the signature.dts we use to store
the EFI capsule certificates
lowest_supported_version -> same here, put it into the .dts file
last_attempt_version -> This is essentially the fw_version *after* the
upgrade is successful
last_attempt_status -> CapsuleXXXX holds that result for us and we
already have code to parse it.

This shields further against someone trying to manipulate
lowest_supported_version, in case someone uses this for rollback
protection checks.

Can we modify this patch accordingly?

Regards
/Ilias




On Tue, 28 Mar 2023 at 14:53, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Kojima-san,
>
> On Thu, Mar 23, 2023 at 08:09:01PM +0900, Masahisa Kojima wrote:
> > Firmware version management is not implemented in the current
> > FMP protocol.
> > EDK II reference implementation capsule generation script inserts
> > the FMP Payload Header right before the payload, it contains the
> > firmware version and lowest supported version.
> >
> > This commit utilizes the FMP Payload Header, reads the header and
> > stores the firmware version, lowest supported version,
> > last attempt version and last attempt status into "FmpStateXXXX"
> > EFI non-volatile variable. XXXX indicates the image index,
> > since FMP protocol handles multiple image indexes.
> >
> > This change is compatible with the existing FMP implementation.
> > This change does not mandate the FMP Payload Header.
> > If no FMP Payload Header is found in the capsule file, fw_version,
> > lowest supported version, last attempt version and last attempt
> > status is 0 and this is the same behavior as existing FMP
> > implementation.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > ---