* [PATCH 0/2] crypto: fix handling & reporting of errors when erasing keyslots @ 2026-10-23 14:13 Daniel P. Berrangé 2026-10-23 14:13 ` [PATCH 1/2] crypto: avoid overwriting Error object erasing keyslot Daniel P. Berrangé 2026-10-23 14:13 ` [PATCH 2/2] crypto: add newline in error hint messages Daniel P. Berrangé 0 siblings, 2 replies; 6+ messages in thread From: Daniel P. Berrangé @ 2026-10-23 14:13 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel P. Berrangé Daniel P. Berrangé (2): crypto: avoid overwriting Error object erasing keyslot crypto: add newline in error hint messages crypto/block-luks.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) -- 2.56.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] crypto: avoid overwriting Error object erasing keyslot 2026-10-23 14:13 [PATCH 0/2] crypto: fix handling & reporting of errors when erasing keyslots Daniel P. Berrangé @ 2026-10-23 14:13 ` Daniel P. Berrangé 2026-10-23 20:08 ` marcandre.lureau 2026-10-23 14:13 ` [PATCH 2/2] crypto: add newline in error hint messages Daniel P. Berrangé 1 sibling, 1 reply; 6+ messages in thread From: Daniel P. Berrangé @ 2026-10-23 14:13 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel P. Berrangé When erasing a keyslot we deliberately ignore some errors and carry on trying to erase as much as possible. This results in overwriting Error objects and triggering an assertion. Don't pass in the &local_err, if errp is already set. Fixes: https://gitlab.com/qemu-project/qemu/-/work_items/4697 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- crypto/block-luks.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index 0926ad28f0c..8a1df846a14 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -1152,22 +1152,29 @@ qcrypto_block_luks_erase_key(QCryptoBlock *block, if (ret < 0) { error_propagate(errp, local_err); + local_err = NULL; } + /* * Now try to erase the key material, even if the header * update failed */ for (i = 0; i < QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS; i++) { if (qcrypto_random_bytes(garbagesplitkey, - splitkeylen, &local_err) < 0) { + splitkeylen, + errp && *errp ? NULL : &local_err) < 0) { /* * If we failed to get the random data, still write * at least zeros to the key slot at least once */ - error_propagate(errp, local_err); + if (local_err) { + error_propagate(errp, local_err); + } if (i > 0) { return -1; + } else { + local_err = NULL; } } if (writefunc(block, @@ -1175,8 +1182,10 @@ qcrypto_block_luks_erase_key(QCryptoBlock *block, garbagesplitkey, splitkeylen, opaque, - &local_err) < 0) { - error_propagate(errp, local_err); + errp && *errp ? NULL : &local_err) < 0) { + if (local_err) { + error_propagate(errp, local_err); + } return -1; } } -- 2.56.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] crypto: avoid overwriting Error object erasing keyslot 2026-10-23 14:13 ` [PATCH 1/2] crypto: avoid overwriting Error object erasing keyslot Daniel P. Berrangé @ 2026-10-23 20:08 ` marcandre.lureau 2026-10-26 5:20 ` Markus Armbruster 0 siblings, 1 reply; 6+ messages in thread From: marcandre.lureau @ 2026-10-23 20:08 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel > When erasing a keyslot we deliberately ignore some errors and > carry on trying to erase as much as possible. This results in > overwriting Error objects and triggering an assertion. Don't > pass in the &local_err, if errp is already set. > > Fixes: https://gitlab.com/qemu-project/qemu/-/work_items/4697 > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > Message-ID: <20261023141303.163893-2-berrange@redhat.com> > --- > crypto/block-luks.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/crypto/block-luks.c b/crypto/block-luks.c > index 0926ad28f0cd..8a1df846a146 100644 > --- a/crypto/block-luks.c > +++ b/crypto/block-luks.c > @@ -1152,22 +1152,29 @@ qcrypto_block_luks_erase_key(QCryptoBlock *block, > > if (ret < 0) { > error_propagate(errp, local_err); > + local_err = NULL; > } > + > /* > * Now try to erase the key material, even if the header > * update failed > */ > for (i = 0; i < QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS; i++) { > if (qcrypto_random_bytes(garbagesplitkey, > - splitkeylen, &local_err) < 0) { > + splitkeylen, > + errp && *errp ? NULL : &local_err) < 0) { I think the current code was simpler. error_propagate() will simpliy discard local_err if errp is already set. It is just missing "local_err = NULL;" > /* > * If we failed to get the random data, still write > * at least zeros to the key slot at least once > */ > - error_propagate(errp, local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + } > > if (i > 0) { > return -1; > + } else { > + local_err = NULL; > } > } > if (writefunc(block, > @@ -1175,8 +1182,10 @@ qcrypto_block_luks_erase_key(QCryptoBlock *block, > garbagesplitkey, > splitkeylen, > opaque, > - &local_err) < 0) { > - error_propagate(errp, local_err); > + errp && *errp ? NULL : &local_err) < 0) { > + if (local_err) { > + error_propagate(errp, local_err); > + } Same, I dont see why that's necessary -- Marc-André Lureau <marcandre.lureau@redhat.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] crypto: avoid overwriting Error object erasing keyslot 2026-10-23 20:08 ` marcandre.lureau @ 2026-10-26 5:20 ` Markus Armbruster 0 siblings, 0 replies; 6+ messages in thread From: Markus Armbruster @ 2026-10-26 5:20 UTC (permalink / raw) To: marcandre.lureau; +Cc: Daniel P. Berrangé, qemu-devel marcandre.lureau@redhat.com writes: >> When erasing a keyslot we deliberately ignore some errors and >> carry on trying to erase as much as possible. This results in >> overwriting Error objects and triggering an assertion. Don't >> pass in the &local_err, if errp is already set. >> >> Fixes: https://gitlab.com/qemu-project/qemu/-/work_items/4697 >> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >> Message-ID: <20261023141303.163893-2-berrange@redhat.com> >> --- >> crypto/block-luks.c | 17 +++++++++++++---- >> 1 file changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/crypto/block-luks.c b/crypto/block-luks.c >> index 0926ad28f0cd..8a1df846a146 100644 >> --- a/crypto/block-luks.c >> +++ b/crypto/block-luks.c >> @@ -1152,22 +1152,29 @@ qcrypto_block_luks_erase_key(QCryptoBlock *block, >> >> if (ret < 0) { >> error_propagate(errp, local_err); >> + local_err = NULL; >> } >> + >> /* >> * Now try to erase the key material, even if the header >> * update failed >> */ >> for (i = 0; i < QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS; i++) { >> if (qcrypto_random_bytes(garbagesplitkey, >> - splitkeylen, &local_err) < 0) { >> + splitkeylen, >> + errp && *errp ? NULL : &local_err) < 0) { > > I think the current code was simpler. error_propagate() will simpliy discard local_err if errp is already set. It is just missing "local_err = NULL;" Moreover, *errp is wrong without ERRP_GUARD(). [...] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] crypto: add newline in error hint messages 2026-10-23 14:13 [PATCH 0/2] crypto: fix handling & reporting of errors when erasing keyslots Daniel P. Berrangé 2026-10-23 14:13 ` [PATCH 1/2] crypto: avoid overwriting Error object erasing keyslot Daniel P. Berrangé @ 2026-10-23 14:13 ` Daniel P. Berrangé 2026-10-23 20:08 ` marcandre.lureau 1 sibling, 1 reply; 6+ messages in thread From: Daniel P. Berrangé @ 2026-10-23 14:13 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel P. Berrangé The error_append_hint() API contract requires messages to end with a newline character. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- crypto/block-luks.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index 8a1df846a14..bdfc85d15ca 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -1681,7 +1681,7 @@ qcrypto_block_luks_amend_add_keyslot(QCryptoBlock *block, if (qcrypto_block_luks_find_key(block, old_password, master_key, readfunc, opaque, errp) < 0) { - error_append_hint(errp, "Failed to retrieve the master key"); + error_append_hint(errp, "Failed to retrieve the master key\n"); return -1; } @@ -1694,7 +1694,7 @@ qcrypto_block_luks_amend_add_keyslot(QCryptoBlock *block, /* Now set the new keyslots */ if (qcrypto_block_luks_store_key(block, keyslot, new_password, master_key, iter_time, writefunc, opaque, errp)) { - error_append_hint(errp, "Failed to write to keyslot %i", keyslot); + error_append_hint(errp, "Failed to write to keyslot %i\n", keyslot); return -1; } return 0; @@ -1792,7 +1792,7 @@ qcrypto_block_luks_amend_erase_keyslots(QCryptoBlock *block, if (qcrypto_block_luks_erase_key(block, keyslot, writefunc, opaque, errp)) { - error_append_hint(errp, "Failed to erase keyslot %i", keyslot); + error_append_hint(errp, "Failed to erase keyslot %i\n", keyslot); return -1; } @@ -1845,7 +1845,7 @@ qcrypto_block_luks_amend_erase_keyslots(QCryptoBlock *block, } if (qcrypto_block_luks_erase_key(block, i, writefunc, opaque, errp)) { - error_append_hint(errp, "Failed to erase keyslot %zu", i); + error_append_hint(errp, "Failed to erase keyslot %zu\n", i); return -1; } } -- 2.56.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] crypto: add newline in error hint messages 2026-10-23 14:13 ` [PATCH 2/2] crypto: add newline in error hint messages Daniel P. Berrangé @ 2026-10-23 20:08 ` marcandre.lureau 0 siblings, 0 replies; 6+ messages in thread From: marcandre.lureau @ 2026-10-23 20:08 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel On Fri, 23 Oct 2026 15:13:03 +0100, Daniel P. Berrangé <berrange@redhat.com> wrote: > The error_append_hint() API contract requires messages to end with > a newline character. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> -- Marc-André Lureau <marcandre.lureau@redhat.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-26 5:20 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-23 14:13 [PATCH 0/2] crypto: fix handling & reporting of errors when erasing keyslots Daniel P. Berrangé 2026-10-23 14:13 ` [PATCH 1/2] crypto: avoid overwriting Error object erasing keyslot Daniel P. Berrangé 2026-10-23 20:08 ` marcandre.lureau 2026-10-26 5:20 ` Markus Armbruster 2026-10-23 14:13 ` [PATCH 2/2] crypto: add newline in error hint messages Daniel P. Berrangé 2026-10-23 20:08 ` marcandre.lureau
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.