[PATCH 2/3] fit: Use hash.c to call SHA code

[Joel Stanley]

On Wed, 17 Feb 2021 at 05:04, AKASHI Takahiro
<takahiro.akashi at linaro.org> wrote:
>
> Simon,
>
> # This is not a direct comment on this patch.
>
> On Wed, Feb 17, 2021 at 01:50:41PM +1030, Joel Stanley wrote:
> > Currently the FIT hashing will call directly into the SHA algorithms to
> > get a hash.
> >
> > This moves the fit code to use hash_lookup_algo, giving a common
> > entrypoint into the hashing code and removing the duplicated algorithm
> > look up. It also allows the use of hardware acceleration if configured.
> >
> > Signed-off-by: Joel Stanley <joel at jms.id.au>
> > ---
> >  common/image-fit.c | 34 ++++++++--------------------------
> >  1 file changed, 8 insertions(+), 26 deletions(-)
> >
> > diff --git a/common/image-fit.c b/common/image-fit.c
> > index 28b3d2b19111..3451cdecc95b 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -1210,37 +1210,19 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp)
> >   *     0, on success
> >   *    -1, when algo is unsupported
> >   */
> > -int calculate_hash(const void *data, int data_len, const char *algo,
> > +int calculate_hash(const void *data, int data_len, const char *algo_name,
> >                       uint8_t *value, int *value_len)
> >  {
> > -     if (IMAGE_ENABLE_CRC32 && strcmp(algo, "crc32") == 0) {
> > -             *((uint32_t *)value) = crc32_wd(0, data, data_len,
> > -                                                     CHUNKSZ_CRC32);
> > -             *((uint32_t *)value) = cpu_to_uimage(*((uint32_t *)value));
> > -             *value_len = 4;
> > -     } else if (IMAGE_ENABLE_SHA1 && strcmp(algo, "sha1") == 0) {
> > -             sha1_csum_wd((unsigned char *)data, data_len,
> > -                          (unsigned char *)value, CHUNKSZ_SHA1);
> > -             *value_len = 20;
> > -     } else if (IMAGE_ENABLE_SHA256 && strcmp(algo, "sha256") == 0) {
> > -             sha256_csum_wd((unsigned char *)data, data_len,
> > -                            (unsigned char *)value, CHUNKSZ_SHA256);
> > -             *value_len = SHA256_SUM_LEN;
> > -     } else if (IMAGE_ENABLE_SHA384 && strcmp(algo, "sha384") == 0) {
> > -             sha384_csum_wd((unsigned char *)data, data_len,
> > -                            (unsigned char *)value, CHUNKSZ_SHA384);
> > -             *value_len = SHA384_SUM_LEN;
> > -     } else if (IMAGE_ENABLE_SHA512 && strcmp(algo, "sha512") == 0) {
> > -             sha512_csum_wd((unsigned char *)data, data_len,
> > -                            (unsigned char *)value, CHUNKSZ_SHA512);
> > -             *value_len = SHA512_SUM_LEN;
> > -     } else if (IMAGE_ENABLE_MD5 && strcmp(algo, "md5") == 0) {
> > -             md5_wd((unsigned char *)data, data_len, value, CHUNKSZ_MD5);
> > -             *value_len = 16;
> > -     } else {
> > +     struct hash_algo *algo;
> > +
> > +     if (hash_lookup_algo(algo_name, &algo)) {
> >               debug("Unsupported hash alogrithm\n");
> >               return -1;
> >       }
> > +
> > +     algo->hash_func_ws(data, data_len, value, algo->chunk_size);
> > +     *value_len = algo->digest_size;
>
> With this patch applied, there co-exists a very similar, hence
> confusing function, hash_calculate(), in rsa-checksum.c (now checksum.c?).
> If there is no particular reason for those two functions,
> we'd better unify them?

hash_calculate is doing a progressive hash over a count of regions.
This code is hashing a single chunk of data.

I agree the naming could be improved to make this clearer.

Cheers,

Joel


>
> -Takahiro Akashi
>
>
> >       return 0;
> >  }
> >
> > --
> > 2.30.0
> >