| Message ID | 20241010162024.988284-9-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/17] crypto: accumulative hashing API | expand |
On 10/10/2024 18.20, Daniel P. Berrangé wrote: > From: Alejandro Zeise <alejandro.zeise@seagate.com> > > Changes the public hash API implementation to support accumulative hashing. > > Implementations for the public functions are added to call the new > driver functions that implement context creation, updating, > finalization, and destruction. > > Additionally changes the "shortcut" functions to use these 4 new core > functions. > > Signed-off-by: Alejandro Zeise <alejandro.zeise@seagate.com> > [ clg: - Reworked qcrypto_hash_bytesv() error handling > - Used hash->driver int qcrypto_hash_new(), qcrypto_hash_free() > qcrypto_hash_updatev() > - Introduced qcrypto_hash_supports() check in > qcrypto_hash_new() > - Introduced g_autofree variables in qcrypto_hash_finalize_digest() > and qcrypto_hash_finalize_base64() > - Re-arrranged code in qcrypto_hash_digestv() and > qcrypto_hash_digest() > - Checkpatch fixes ] > Signed-off-by: Cédric Le Goater <clg@redhat.com> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > crypto/hash.c | 161 ++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 131 insertions(+), 30 deletions(-) Hi, something recently broke qemu-iotest 081 in raw mode and bisecting it pointed me to this commit here. cd tests/qemu-iotests/ ; ./check -raw 081 ; cd ../../ [...] 081 fail [21:07:40] [21:07:42] 1.4s (last: 1.4s) output mismatch (see /home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/raw-file-081/081.out.bad) --- /home/thuth/devel/qemu/tests/qemu-iotests/081.out +++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/raw-file-081/081.out.bad @@ -31,7 +31,6 @@ {"return": {}} {"return": {}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "QUORUM_REPORT_BAD", "data": {"node-name": "drive2", "sectors-count": 20480, "sector-num": 0, "type": "read"}} read 10485760/10485760 bytes at offset 0 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) {"return": ""} @@ -44,6 +43,7 @@ 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) == checking that quorum has corrected the corrupted file == +Pattern verification failed at offset 0, 10485760 bytes read 10485760/10485760 bytes at offset 0 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) @@ -63,7 +63,6 @@ } -device virtio-scsi,id=scsi -device scsi-hd,id=quorum-drive,bus=scsi.0,drive=quorum QMP_VERSION {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "QUORUM_REPORT_BAD", "data": {"node-name": "file2", "sectors-count": 20480, "sector-num": 0, "type": "read"}} read 10485760/10485760 bytes at offset 0 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) {"return": ""} @@ -71,6 +70,7 @@ {"return": {}} -- checking that the image has been corrected -- +Pattern verification failed at offset 0, 10485760 bytes read 10485760/10485760 bytes at offset 0 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) @@ -81,7 +81,9 @@ 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) == checking that quorum is broken == -read failed: Input/output error +Pattern verification failed at offset 0, 10485760 bytes +read 10485760/10485760 bytes at offset 0 +10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) == checking the blkverify mode with broken content == quorum: offset=0 bytes=10485760 contents mismatch at offset 0 Failures: 081 Failed 1 of 1 iotests Could you please have a look? Thanks, Thomas
On Tue, Oct 22, 2024 at 09:10:50PM +0200, Thomas Huth wrote: > On 10/10/2024 18.20, Daniel P. Berrangé wrote: > > From: Alejandro Zeise <alejandro.zeise@seagate.com> > > > > Changes the public hash API implementation to support accumulative hashing. > > > > Implementations for the public functions are added to call the new > > driver functions that implement context creation, updating, > > finalization, and destruction. > > > > Additionally changes the "shortcut" functions to use these 4 new core > > functions. > > > > Signed-off-by: Alejandro Zeise <alejandro.zeise@seagate.com> > > [ clg: - Reworked qcrypto_hash_bytesv() error handling > > - Used hash->driver int qcrypto_hash_new(), qcrypto_hash_free() > > qcrypto_hash_updatev() > > - Introduced qcrypto_hash_supports() check in > > qcrypto_hash_new() > > - Introduced g_autofree variables in qcrypto_hash_finalize_digest() > > and qcrypto_hash_finalize_base64() > > - Re-arrranged code in qcrypto_hash_digestv() and > > qcrypto_hash_digest() > > - Checkpatch fixes ] > > Signed-off-by: Cédric Le Goater <clg@redhat.com> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > crypto/hash.c | 161 ++++++++++++++++++++++++++++++++++++++++---------- > > 1 file changed, 131 insertions(+), 30 deletions(-) > > Hi, > > something recently broke qemu-iotest 081 in raw mode and bisecting it > pointed me to this commit here. snip > Could you please have a look? This is fixed by my current pending pull request With regards, Daniel
diff --git a/crypto/hash.c b/crypto/hash.c index 4a265582b8..0c8548c568 100644 --- a/crypto/hash.c +++ b/crypto/hash.c @@ -1,6 +1,7 @@ /* * QEMU Crypto hash algorithms * + * Copyright (c) 2024 Seagate Technology LLC and/or its Affiliates * Copyright (c) 2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or @@ -19,6 +20,8 @@ */ #include "qemu/osdep.h" +#include "qapi/error.h" +#include "qapi-types-crypto.h" #include "crypto/hash.h" #include "hashpriv.h" @@ -45,23 +48,18 @@ int qcrypto_hash_bytesv(QCryptoHashAlgo alg, size_t *resultlen, Error **errp) { -#ifdef CONFIG_AF_ALG - int ret; - /* - * TODO: - * Maybe we should treat some afalg errors as fatal - */ - ret = qcrypto_hash_afalg_driver.hash_bytesv(alg, iov, niov, - result, resultlen, - NULL); - if (ret == 0) { - return ret; + g_autoptr(QCryptoHash) ctx = qcrypto_hash_new(alg, errp); + + if (!ctx) { + return -1; + } + + if (qcrypto_hash_updatev(ctx, iov, niov, errp) < 0 || + qcrypto_hash_finalize_bytes(ctx, result, resultlen, errp) < 0) { + return -1; } -#endif - return qcrypto_hash_lib_driver.hash_bytesv(alg, iov, niov, - result, resultlen, - errp); + return 0; } @@ -77,29 +75,130 @@ int qcrypto_hash_bytes(QCryptoHashAlgo alg, return qcrypto_hash_bytesv(alg, &iov, 1, result, resultlen, errp); } +int qcrypto_hash_updatev(QCryptoHash *hash, + const struct iovec *iov, + size_t niov, + Error **errp) +{ + QCryptoHashDriver *drv = hash->driver; + + return drv->hash_update(hash, iov, niov, errp); +} + +int qcrypto_hash_update(QCryptoHash *hash, + const char *buf, + size_t len, + Error **errp) +{ + struct iovec iov = { .iov_base = (char *)buf, .iov_len = len }; + + return qcrypto_hash_updatev(hash, &iov, 1, errp); +} + +QCryptoHash *qcrypto_hash_new(QCryptoHashAlgo alg, Error **errp) +{ + QCryptoHash *hash = NULL; + + if (!qcrypto_hash_supports(alg)) { + error_setg(errp, "Unsupported hash algorithm %s", + QCryptoHashAlgo_str(alg)); + return NULL; + } + +#ifdef CONFIG_AF_ALG + hash = qcrypto_hash_afalg_driver.hash_new(alg, NULL); + if (hash) { + hash->driver = &qcrypto_hash_afalg_driver; + return hash; + } +#endif + + hash = qcrypto_hash_lib_driver.hash_new(alg, errp); + if (!hash) { + return NULL; + } + + hash->driver = &qcrypto_hash_lib_driver; + return hash; +} + +void qcrypto_hash_free(QCryptoHash *hash) +{ + QCryptoHashDriver *drv; + + if (hash) { + drv = hash->driver; + drv->hash_free(hash); + } +} + +int qcrypto_hash_finalize_bytes(QCryptoHash *hash, + uint8_t **result, + size_t *result_len, + Error **errp) +{ + QCryptoHashDriver *drv = hash->driver; + + return drv->hash_finalize(hash, result, result_len, errp); +} + static const char hex[] = "0123456789abcdef"; +int qcrypto_hash_finalize_digest(QCryptoHash *hash, + char **digest, + Error **errp) +{ + int ret; + g_autofree uint8_t *result = NULL; + size_t resultlen = 0; + size_t i; + + ret = qcrypto_hash_finalize_bytes(hash, &result, &resultlen, errp); + if (ret == 0) { + *digest = g_new0(char, (resultlen * 2) + 1); + for (i = 0 ; i < resultlen ; i++) { + (*digest)[(i * 2)] = hex[(result[i] >> 4) & 0xf]; + (*digest)[(i * 2) + 1] = hex[result[i] & 0xf]; + } + (*digest)[resultlen * 2] = '\0'; + } + + return ret; +} + +int qcrypto_hash_finalize_base64(QCryptoHash *hash, + char **base64, + Error **errp) +{ + int ret; + g_autofree uint8_t *result = NULL; + size_t resultlen = 0; + + ret = qcrypto_hash_finalize_bytes(hash, &result, &resultlen, errp); + if (ret == 0) { + *base64 = g_base64_encode(result, resultlen); + } + + return ret; +} + int qcrypto_hash_digestv(QCryptoHashAlgo alg, const struct iovec *iov, size_t niov, char **digest, Error **errp) { - uint8_t *result = NULL; - size_t resultlen = 0; - size_t i; + g_autoptr(QCryptoHash) ctx = qcrypto_hash_new(alg, errp); - if (qcrypto_hash_bytesv(alg, iov, niov, &result, &resultlen, errp) < 0) { + if (!ctx) { return -1; } - *digest = g_new0(char, (resultlen * 2) + 1); - for (i = 0 ; i < resultlen ; i++) { - (*digest)[(i * 2)] = hex[(result[i] >> 4) & 0xf]; - (*digest)[(i * 2) + 1] = hex[result[i] & 0xf]; + if (qcrypto_hash_updatev(ctx, iov, niov, errp) < 0 || + qcrypto_hash_finalize_digest(ctx, digest, errp) < 0) { + return -1; } - (*digest)[resultlen * 2] = '\0'; - g_free(result); + return 0; } @@ -120,15 +219,17 @@ int qcrypto_hash_base64v(QCryptoHashAlgo alg, char **base64, Error **errp) { - uint8_t *result = NULL; - size_t resultlen = 0; + g_autoptr(QCryptoHash) ctx = qcrypto_hash_new(alg, errp); + + if (!ctx) { + return -1; + } - if (qcrypto_hash_bytesv(alg, iov, niov, &result, &resultlen, errp) < 0) { + if (qcrypto_hash_updatev(ctx, iov, niov, errp) < 0 || + qcrypto_hash_finalize_base64(ctx, base64, errp) < 0) { return -1; } - *base64 = g_base64_encode(result, resultlen); - g_free(result); return 0; }