Re: [PATCH 1/4] xen/lib: Export additional sha256 functions

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Tue, May 6, 2025 at 11:17 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> On 06/05/2025 3:05 pm, Andrew Cooper wrote:
> > On 06/05/2025 2:56 pm, Frediano Ziglio wrote:
> >> diff --git a/xen/include/xen/sha2.h b/xen/include/xen/sha2.h
> >> index 47d97fbf01..ea8bad67e4 100644
> >> --- a/xen/include/xen/sha2.h
> >> +++ b/xen/include/xen/sha2.h
> >> @@ -9,6 +9,16 @@
> >>
> >>  #define SHA2_256_DIGEST_SIZE 32
> >>
> >> +struct sha2_256_state {
> >> +    uint32_t state[SHA2_256_DIGEST_SIZE / sizeof(uint32_t)];
> >> +    uint8_t buf[64];
> >> +    size_t count; /* Byte count. */
> >> +};
> >> +
> >> +void sha2_256_init(struct sha2_256_state *s);
> >> +void sha2_256_update(struct sha2_256_state *s, const void *msg,
> >> +                     size_t len);
> >> +void sha2_256_final(struct sha2_256_state *s, void *_dst);
> >>  void sha2_256_digest(uint8_t digest[SHA2_256_DIGEST_SIZE],
> >>                       const void *msg, size_t len);
> > sha2_256_digest() is unlike the others as it holds sha2_256_state
> > internally.  I'd suggest having all of the additions below this point,
> > which group them more nicely.
> >
> > Can fix on commit.  Otherwise LGTM.
>
> Not quite.  Now that sha2_256_final() is exported, it should have a
> proper type for  _dst.  I've folded:
>
> diff --git a/xen/include/xen/sha2.h b/xen/include/xen/sha2.h
> index 0d55fe640431..cb30e8f8d77c 100644
> --- a/xen/include/xen/sha2.h
> +++ b/xen/include/xen/sha2.h
> @@ -21,6 +21,7 @@ struct sha2_256_state {
>  void sha2_256_init(struct sha2_256_state *s);
>  void sha2_256_update(struct sha2_256_state *s, const void *msg,
>                       size_t len);
> -void sha2_256_final(struct sha2_256_state *s, void *_dst);
> +void sha2_256_final(struct sha2_256_state *s,
> +                    uint8_t digest[SHA2_256_DIGEST_SIZE]);
>
>  #endif /* XEN_SHA2_H */
> diff --git a/xen/lib/sha2-256.c b/xen/lib/sha2-256.c
> index 896a257ed9b7..08ef7011a1c3 100644
> --- a/xen/lib/sha2-256.c
> +++ b/xen/lib/sha2-256.c
> @@ -171,9 +171,9 @@ void sha2_256_update(struct sha2_256_state *s, const
> void *msg,
>      memcpy(s->buf + partial, msg, len);
>  }
>
> -void sha2_256_final(struct sha2_256_state *s, void *_dst)
> +void sha2_256_final(struct sha2_256_state *s, uint8_t
> digest[SHA2_256_DIGEST_SIZE])
>  {
> -    uint32_t *dst = _dst;
> +    uint32_t *dst = (uint32_t *)digest;

That is we are never going to support architectures with unaligned
memory access.

>      unsigned int i, partial = s->count & 63;
>
>      /* Start padding */
>
> in too, which is compatible with the rest of the series too.
>
> ~Andrew

I'll send an updated series with these and all the commits (mail server issues).

Frediano