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

[Masahisa Kojima]

Hi Ilias,

On Tue, 28 Mar 2023 at 22:02, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> 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

OK, I will add these two values into .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.

To fill ESRT, I think these two values are not very important.
I will omit these values in my next series.

>
> 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?

Thank you for your comment.
I will revise this series.

Regards,
Masahisa Kojima

>
> 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>
> > > ---