1. Patchew
  2. Xen
  3. [PATCH 0/4] Allows Secure Boot for Kexec
  4. View patch

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

Frediano Ziglio posted 4 patches 5 months, 4 weeks ago
Only 2 patches received!
There is a newer version of this series
[PATCH 1/4] xen/lib: Export additional sha256 functions
Posted by Frediano Ziglio 5 months, 4 weeks ago 
From: Ross Lagerwall <ross.lagerwall@citrix.com>

In future, some code needs to generate a digest over several separate buffers
so export the necessary functions to do so.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/include/xen/sha2.h | 10 ++++++++++
 xen/lib/sha2-256.c     | 14 ++++----------
 2 files changed, 14 insertions(+), 10 deletions(-)

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);
 
diff --git a/xen/lib/sha2-256.c b/xen/lib/sha2-256.c
index 19e8252188..896a257ed9 100644
--- a/xen/lib/sha2-256.c
+++ b/xen/lib/sha2-256.c
@@ -10,12 +10,6 @@
 #include <xen/string.h>
 #include <xen/unaligned.h>
 
-struct sha2_256_state {
-    uint32_t state[SHA2_256_DIGEST_SIZE / sizeof(uint32_t)];
-    uint8_t buf[64];
-    size_t count; /* Byte count. */
-};
-
 static uint32_t choose(uint32_t x, uint32_t y, uint32_t z)
 {
     return z ^ (x & (y ^ z));
@@ -131,7 +125,7 @@ static void sha2_256_transform(uint32_t *state, const void *_input)
     state[4] += e; state[5] += f; state[6] += g; state[7] += h;
 }
 
-static void sha2_256_init(struct sha2_256_state *s)
+void sha2_256_init(struct sha2_256_state *s)
 {
     *s = (struct sha2_256_state){
         .state = {
@@ -147,8 +141,8 @@ static void sha2_256_init(struct sha2_256_state *s)
     };
 }
 
-static void sha2_256_update(struct sha2_256_state *s, const void *msg,
-                            size_t len)
+void sha2_256_update(struct sha2_256_state *s, const void *msg,
+                     size_t len)
 {
     unsigned int partial = s->count & 63;
 
@@ -177,7 +171,7 @@ static void sha2_256_update(struct sha2_256_state *s, const void *msg,
     memcpy(s->buf + partial, msg, len);
 }
 
-static void sha2_256_final(struct sha2_256_state *s, void *_dst)
+void sha2_256_final(struct sha2_256_state *s, void *_dst)
 {
     uint32_t *dst = _dst;
     unsigned int i, partial = s->count & 63;
-- 
2.43.0
Re: [PATCH 1/4] xen/lib: Export additional sha256 functions
Posted by Andrew Cooper 5 months, 4 weeks ago 
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.

~Andrew
Re: [PATCH 1/4] xen/lib: Export additional sha256 functions
Posted by Jan Beulich 5 months, 3 weeks ago 
On 06.05.2025 16:05, 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.

I notice this was committed, but isn't this introducing new Misra
violations (extern functions without external callers)?

Jan
Re: [PATCH 1/4] xen/lib: Export additional sha256 functions
Posted by Andrew Cooper 5 months, 4 weeks ago 
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;
     unsigned int i, partial = s->count & 63;
 
     /* Start padding */

in too, which is compatible with the rest of the series too.

~Andrew
Re: [PATCH 1/4] xen/lib: Export additional sha256 functions
Posted by Frediano Ziglio 5 months, 4 weeks ago 
On Tue, May 6, 2025 at 11:17 PM Andrew Cooper <andrew.cooper3@citrix.com> 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
Re: [PATCH 1/4] xen/lib: Export additional sha256 functions
Posted by Andrew Cooper 5 months, 3 weeks ago 
On 07/05/2025 6:26 am, Frediano Ziglio wrote:
> On Tue, May 6, 2025 at 11:17 PM Andrew Cooper <andrew.cooper3@citrix.com> 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.

Note how it's written to.

    /* Store state in digest */
    for ( i = 0; i < 8; i++ )
        put_unaligned_be32(s->state[i], &dst[i]);

It works just fine on misaligned buffers.

~Andrew

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.