Re: [PATCH v5 05/16] crypto/hash-nettle: Implement new hash API

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Oct 10, 2024 at 11:49:22AM +0100, Daniel P. Berrangé wrote:
> On Tue, Oct 08, 2024 at 09:57:12AM +0200, Cédric Le Goater wrote:
> > From: Alejandro Zeise <alejandro.zeise@seagate.com>
> > 
> > Implements the new hashing API in the nettle hash driver.
> > Supports creating/destroying a context, updating the context
> > with input data and obtaining an output hash.
> > 
> > Signed-off-by: Alejandro Zeise <alejandro.zeise@seagate.com>
> > [ clg: - Dropped qcrypto_hash_supports() in qcrypto_nettle_hash_new() ]
> > Signed-off-by: Cédric Le Goater <clg@redhat.com>
> > ---
> >  crypto/hash-nettle.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> > 
> > diff --git a/crypto/hash-nettle.c b/crypto/hash-nettle.c
> > index 8b08a9c67531..07e18ce26cba 100644
> > --- a/crypto/hash-nettle.c
> > +++ b/crypto/hash-nettle.c
> 
> > +static
> > +int qcrypto_nettle_hash_update(QCryptoHash *hash,
> > +                               const struct iovec *iov,
> > +                               size_t niov,
> > +                               Error **errp)
> > +{
> > +    union qcrypto_hash_ctx *ctx = hash->opaque;
> > +
> > +    for (int i = 0; i < niov; i++) {
> > +        /*
> > +         * Some versions of nettle have functions
> > +         * declared with 'int' instead of 'size_t'
> > +         * so to be safe avoid writing more than
> > +         * UINT_MAX bytes at a time
> > +         */
> 
> This is copying the pre-existing code, and I've just
> realized this is now obsolete. We bumped min nettle
> to 3.4 ages ago and >= 3.0 is using size_t, so we can
> simplify now.
> 
> No need to change this though. I'll do that as a 
> distinct patch to make it a visible change.
> 
> > +        size_t len = iov[i].iov_len;
> > +        uint8_t *base = iov[i].iov_base;
> > +        while (len) {
> > +            size_t shortlen = MIN(len, UINT_MAX);
> > +            qcrypto_hash_alg_map[hash->alg].write(ctx, len, base);
> > +            len -= shortlen;
> > +            base += len;
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}

I'm going to add the following patch on the end of your series:

commit 638ae66c176f8077ccaac10028812e177224a99f
Author: Daniel P. Berrangé <berrange@redhat.com>
Date:   Thu Oct 10 12:40:49 2024 +0100

    crypto: drop obsolete back compat logic for old nettle
    
    The nettle 2.x series declared all the hash functions with 'int' for
    the data size. Since we dropped support for anything older than 3.4
    we can assume nettle is using 'size_t' and thus avoid the back compat
    looping logic.
    
    Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/crypto/hash-nettle.c b/crypto/hash-nettle.c
index 570ce8a645..3b847aa60e 100644
--- a/crypto/hash-nettle.c
+++ b/crypto/hash-nettle.c
@@ -135,20 +135,9 @@ int qcrypto_nettle_hash_update(QCryptoHash *hash,
     union qcrypto_hash_ctx *ctx = hash->opaque;
 
     for (int i = 0; i < niov; i++) {
-        /*
-         * Some versions of nettle have functions
-         * declared with 'int' instead of 'size_t'
-         * so to be safe avoid writing more than
-         * UINT_MAX bytes at a time
-         */
-        size_t len = iov[i].iov_len;
-        uint8_t *base = iov[i].iov_base;
-        while (len) {
-            size_t shortlen = MIN(len, UINT_MAX);
-            qcrypto_hash_alg_map[hash->alg].write(ctx, len, base);
-            len -= shortlen;
-            base += len;
-        }
+        qcrypto_hash_alg_map[hash->alg].write(ctx,
+                                              iov[i].iov_len,
+                                              iov[i].iov_base);
     }
 
     return 0;



With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|