Skip to content

Commit

Permalink
crypto: distinguish between main loop and I/O in block_crypto_amend_o…
Browse files Browse the repository at this point in the history
…ptions_generic_luks

block_crypto_amend_options_generic_luks uses the block layer
permission API, therefore it should be called with the BQL held.

However, the same function is being called by two BlockDriver
callbacks: bdrv_amend_options (under BQL) and bdrv_co_amend (I/O).

The latter is I/O because it is invoked by block/amend.c's
blockdev_amend_run(), a .run callback of the amend JobDriver.

Therefore we want to change this function to still perform
the permission check, but making sure it is done under BQL regardless
of the caller context.

Remove the permission check in block_crypto_amend_options_generic_luks()
and:
- in block_crypto_amend_options_luks() (BQL case, called by
  .bdrv_amend_options()), reuse helper functions
  block_crypto_amend_{prepare/cleanup} that take care of checking
  permissions.

- for block_crypto_co_amend_luks() (I/O case, called by
  .bdrv_co_amend()), don't check for permissions but delegate
  .bdrv_amend_pre_run() and .bdrv_amend_clean() to do it,
  performing these checks before and after the job runs in its aiocontext.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20220209105452.1694545-3-eesposit@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
esposem authored and kevmw committed Mar 4, 2022
1 parent c1019d1 commit dae8492
Showing 1 changed file with 15 additions and 20 deletions.
35 changes: 15 additions & 20 deletions block/crypto.c
Expand Up @@ -815,30 +815,17 @@ block_crypto_amend_options_generic_luks(BlockDriverState *bs,
Error **errp)
{
BlockCrypto *crypto = bs->opaque;
int ret;

assert(crypto);
assert(crypto->block);

/* apply for exclusive read/write permissions to the underlying file*/
crypto->updating_keys = true;
ret = bdrv_child_refresh_perms(bs, bs->file, errp);
if (ret) {
goto cleanup;
}

ret = qcrypto_block_amend_options(crypto->block,
block_crypto_read_func,
block_crypto_write_func,
bs,
amend_options,
force,
errp);
cleanup:
/* release exclusive read/write permissions to the underlying file*/
crypto->updating_keys = false;
bdrv_child_refresh_perms(bs, bs->file, errp);
return ret;
return qcrypto_block_amend_options(crypto->block,
block_crypto_read_func,
block_crypto_write_func,
bs,
amend_options,
force,
errp);
}

static int
Expand All @@ -864,8 +851,16 @@ block_crypto_amend_options_luks(BlockDriverState *bs,
if (!amend_options) {
goto cleanup;
}

ret = block_crypto_amend_prepare(bs, errp);
if (ret) {
goto perm_cleanup;
}
ret = block_crypto_amend_options_generic_luks(bs, amend_options,
force, errp);

perm_cleanup:
block_crypto_amend_cleanup(bs);
cleanup:
qapi_free_QCryptoBlockAmendOptions(amend_options);
return ret;
Expand Down

0 comments on commit dae8492

Please sign in to comment.