Daniel P. Berrangé <[email protected]> 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é <[email protected]>
> ---
> crypto/block-luks.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index 0926ad28f0c..277a5110607 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -1152,6 +1152,7 @@ qcrypto_block_luks_erase_key(QCryptoBlock *block,
{
QCryptoBlockLUKS *luks = block->opaque;
QCryptoBlockLUKSKeySlot *slot;
g_autofree uint8_t *garbagesplitkey = NULL;
size_t splitkeylen;
size_t i;
Error *local_err = NULL;
int ret;
assert(slot_idx < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
slot = &luks->header.key_slots[slot_idx];
splitkeylen = luks->header.master_key_len * slot->stripes;
assert(splitkeylen > 0);
garbagesplitkey = g_new0(uint8_t, splitkeylen);
/* Reset the key slot header */
memset(slot->salt, 0, QCRYPTO_BLOCK_LUKS_SALT_LEN);
slot->iterations = 0;
slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
ret = qcrypto_block_luks_store_header(block, writefunc,
opaque, &local_err);
Why not pass @errp here, and ...
>
> if (ret < 0) {
> error_propagate(errp, local_err);
... drop this line?
> + local_err = NULL;
> }
Suggest a blank line here.
Note for later: @errp may be non-null now.
> /*
> * Now try to erase the key material, even if the header
> @@ -1169,6 +1170,7 @@ qcrypto_block_luks_erase_key(QCryptoBlock *block,
* update failed
*/
for (i = 0; i < QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS; i++) {
if (qcrypto_random_bytes(garbagesplitkey,
splitkeylen, &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 @errp is still null, store this error, else ignore it.
> if (i > 0) {
> return -1;
Stop when erasing non-first key slot failed.
> }
> + local_err = NULL;
> }
> if (writefunc(block,
> slot->key_offset_sector *
> QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
garbagesplitkey,
splitkeylen,
opaque,
&local_err) < 0) {
error_propagate(errp, local_err);
We do need error_propagate() here, because @errp can be non-null.
return -1;
}
}
return ret;
}
Nothing is wrong, so
Reviewed-by: Markus Armbruster <[email protected]>
