| Message ID | 20241008075724.2772149-6-clg@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hw/misc/aspeed_hace: Fix SG Accumulative Hash Calculations | expand |
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; > +} With regards, Daniel
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
On 10/10/24 13:43, Daniel P. Berrangé wrote: > 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; Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C.
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 @@ -1,6 +1,7 @@ /* * QEMU Crypto hash algorithms * + * Copyright (c) 2024 Seagate Technology LLC and/or its Affiliates * Copyright (c) 2016 Red Hat, Inc. * * This library is free software; you can redistribute it and/or @@ -155,7 +156,76 @@ qcrypto_nettle_hash_bytesv(QCryptoHashAlgo alg, return 0; } +static +QCryptoHash *qcrypto_nettle_hash_new(QCryptoHashAlgo alg, Error **errp) +{ + QCryptoHash *hash; + + hash = g_new(QCryptoHash, 1); + hash->alg = alg; + hash->opaque = g_new(union qcrypto_hash_ctx, 1); + + qcrypto_hash_alg_map[alg].init(hash->opaque); + return hash; +} + +static +void qcrypto_nettle_hash_free(QCryptoHash *hash) +{ + union qcrypto_hash_ctx *ctx = hash->opaque; + + g_free(ctx); + g_free(hash); +} + +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 + */ + 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; +} + +static +int qcrypto_nettle_hash_finalize(QCryptoHash *hash, + uint8_t **result, + size_t *result_len, + Error **errp) +{ + union qcrypto_hash_ctx *ctx = hash->opaque; + + *result_len = qcrypto_hash_alg_map[hash->alg].len; + *result = g_new(uint8_t, *result_len); + + qcrypto_hash_alg_map[hash->alg].result(ctx, *result_len, *result); + + return 0; +} QCryptoHashDriver qcrypto_hash_lib_driver = { .hash_bytesv = qcrypto_nettle_hash_bytesv, + .hash_new = qcrypto_nettle_hash_new, + .hash_update = qcrypto_nettle_hash_update, + .hash_finalize = qcrypto_nettle_hash_finalize, + .hash_free = qcrypto_nettle_hash_free, };