[PATCH v10 10/15] FWU: Add support for the FWU Multi Bank Update feature

[Jassi Brar]

On Tue, Sep 27, 2022 at 2:22 AM Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> On Mon, 26 Sept 2022 at 20:24, Jassi Brar <jassisinghbrar at gmail.com> wrote:
> >
> > On Mon, Sep 26, 2022 at 4:01 AM Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > On Mon, 26 Sept 2022 at 08:25, Jassi Brar <jassisinghbrar at gmail.com> wrote:
> >
> > > > .....
> > > > >
> > > > > +static __maybe_unused efi_status_t fwu_post_update_process(bool fw_accept_os)
> > > > > +{
> > > > > +       int status;
> > > > > +       u32 update_index;
> > > > > +       efi_status_t ret;
> > > > > +
> > > > > +       status = fwu_plat_get_update_index(&update_index);
> > > > > +       if (status < 0) {
> > > > > +               log_err("Failed to get the FWU update_index value\n");
> > > > > +               return EFI_DEVICE_ERROR;
> > > > > +       }
> > > > > +
> > > > > +       /*
> > > > > +        * All the capsules have been updated successfully,
> > > > > +        * update the FWU metadata.
> > > > > +        */
> > > > > +       log_debug("Update Complete. Now updating active_index to %u\n",
> > > > > +                 update_index);
> > > > > +       status = fwu_update_active_index(update_index);
> > > > >
> > > > Do we want to check if all images in the bank are updated via capsules
> > > > before switching the bank?
> > >
> > > This function does get called only when the update status for every
> > > capsule is a success. Even if one of the capsules does not get
> > > updated, the active index will not get updated.
> > >
> > .... but we don't check if the capsule for each image in the bank is
> > provided for update.
>
> Yes, we have had this discussion earlier. Neither the FWU spec, nor
> the capsule update spec in UEFI puts that restriction that all images
> on the platform need to be updated. If a user wants to ensure such a
> behaviour, they may choose some kind of image packaging like FIP or
> FIT which would mean that all the images on the platform are being
> updated. But this is not something to be ensured by the FWU update
> code.
>
> >
> > > >
> > > > A developer will make sure all images are provided in one go, so that
> > > > the switch is successful.
> > > > But a malicious user may force some old vulnerable image back into use
> > > > by updating all but that image.
> > >
> > > That I believe is to be handled through a combination of implementing
> > > a rollback protection mechanism, along with capsule authentication.
> > > These are separate to the implementation of the multi bank updates
> > > that these patches are aiming for.
> > >
> > This sounds like : we don't worry about buffer-overflow
> > vulnerabilities because the system will be secured and hardened by
> > other mechanisms.
>
> Not sure how this is related. The aim of the FWU spec is for providing
> a means for a platform to maintain multiple partitions of images and
> to specify the metadata structure for the bookkeeping of the different
> partitions and images on those partitions. If we need a more secure
> and hardened system, we do have the Dependable Boot spec, which is
> talking precisely about these things. Those are indeed separate
> features or aspects from what the FWU spec is talking about. And this
> patchset is implementing the FWU spec.
>

I am out of ways to explain. Best of luck.