Re: [PATCH v7 06/15] crypto: hash: support hardware-only progressive hashing

On Fri, Aug 14, 2026 at 4:31 PM Simon Glass <[email protected]> wrote:
>
> Hi James,
>
> On 2026-08-13T19:58:53, James Hilliard <[email protected]> wrote:
> > crypto: hash: support hardware-only progressive hashing
> >
> > FIT configuration signatures hash multiple discontiguous regions through
> > the legacy progressive hash interface. This still requires a software
> > implementation even when image hashes use a driver-model hardware
> > provider.
> >
> > Add provider selection for progressive driver-model hashing and use it
> > from hash_calculate(). Allow SPL_SHA256_LEGACY to be disabled explicitly,
> > link the SHA-256 software support only for a selected software backend,
> > and make legacy fallback paths reject an algorithm whose software
> > callbacks are absent.
> >
> > Extend the hash provider-selection test to cover progressive
> > initialization.
>
> That seems like a stale comment, at least for this commit.

Missing tests added in v8:
https://lore.kernel.org/u-boot/[email protected]/

> >
> > This permits SPL to retain SHA-256 FIT support while relying exclusively
> > on a hardware hash provider.
> >
> > Signed-off-by: James Hilliard <[email protected]>
> >
> > boot/image-fit.c                  |  2 +-
> >  common/hash.c                     | 21 +++++++++++++++++++--
> >  drivers/crypto/hash/hash-uclass.c | 31 +++++++++++++++++++++++++++++++
> >  include/u-boot/hash.h             | 14 ++++++++++++++
> >  lib/Makefile                      |  4 ++--
> >  lib/hash-checksum.c               | 28 +++++++++++++++++++++++++++-
> >  lib/mbedtls/Kconfig               |  2 +-
> >  7 files changed, 95 insertions(+), 7 deletions(-)
>
> > diff --git a/lib/hash-checksum.c b/lib/hash-checksum.c
> > @@ -20,13 +22,37 @@ int hash_calculate(const char *name,
> > +             if (!ret) {
> > +                     for (i = 0; i < region_count; i++) {
> > +                             ret = hash_update(dev, ctx, region[i].data,
> > +                                               region[i].size);
> > +                             if (ret)
> > +                                     return ret;
> > +                     }
> > +
> > +                     return hash_finish(dev, ctx, checksum);
> > +             }
>
> If hash_update() fails partway through, the provider's ctx is leaked -
> nothing calls hash_finish() or an equivalent free path. Do you need an
> explicit teardown on the error exit?

Yes. In v8 I added an explicit hash_abort() provider operation. After a
successful hash_init(), the context must be consumed by exactly one
hash_finish() or hash_abort() call.

hash_calculate() now calls hash_abort() after any hash_update() failure
and returns the original update error. The existing progressive providers
were updated with matching abort cleanup.

The new failure-path test verifies that an update error calls
hash_abort() exactly once and does not call hash_finish().

See v8:
https://lore.kernel.org/u-boot/[email protected]/

>
> Reviewed-by: Simon Glass <[email protected]>
>
> Regards,
> Simon