On Tue, Dec 29 2020 at 3:55am -0500,
Satya Tangirala <satyat@xxxxxxxxxx> wrote:
> DM only allows the table to be swapped if the new table's inline encryption
> capabilities are a superset of the old table's. We only check that this
> constraint is true when the table is actually swapped in (in
> dm_swap_table()). But this allows a user to load an unacceptable table
> without any complaint from DM, only for DM to throw an error when the
> device is resumed, and the table is swapped in.
>
> This patch makes DM verify the inline encryption capabilities of the new
> table when the table is loaded. DM continues to verify and use the
> capabilities at the time of table swap, since the capabilities of
> underlying child devices can expand during the time between the table load
> and table swap (which in turn can cause the capabilities of this parent
> device to expand as well).
>
> Signed-off-by: Satya Tangirala <satyat@xxxxxxxxxx>
> ---
> drivers/md/dm-ioctl.c | 8 ++++++++
> drivers/md/dm.c | 25 +++++++++++++++++++++++++
> drivers/md/dm.h | 19 +++++++++++++++++++
> 3 files changed, 52 insertions(+)
>
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 5e306bba4375..055a3c745243 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -1358,6 +1358,10 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si
> goto err_unlock_md_type;
> }
>
> + r = dm_verify_inline_encryption(md, t);
> + if (r)
> + goto err_unlock_md_type;
> +
> if (dm_get_md_type(md) == DM_TYPE_NONE) {
> /* Initial table load: acquire type of table. */
> dm_set_md_type(md, dm_table_get_type(t));
> @@ -2115,6 +2119,10 @@ int __init dm_early_create(struct dm_ioctl *dmi,
> if (r)
> goto err_destroy_table;
>
> + r = dm_verify_inline_encryption(md, t);
> + if (r)
> + goto err_destroy_table;
> +
> md->type = dm_table_get_type(t);
> /* setup md->queue to reflect md's type (may block) */
> r = dm_setup_md_queue(md, t);
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index b8844171d8e4..04322de34d29 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -2094,6 +2094,31 @@ dm_construct_keyslot_manager(struct mapped_device *md, struct dm_table *t)
> return ksm;
> }
>
> +/**
> + * dm_verify_inline_encryption() - Verifies that the current keyslot manager of
> + * the mapped_device can be replaced by the
> + * keyslot manager of a given dm_table.
> + * @md: The mapped_device
> + * @t: The dm_table
> + *
> + * In particular, this function checks that the keyslot manager that will be
> + * constructed for the dm_table will support a superset of the capabilities that
> + * the current keyslot manager of the mapped_device supports.
> + *
> + * Return: 0 if the table's keyslot_manager can replace the current keyslot
> + * manager of the mapped_device. Negative value otherwise.
> + */
> +int dm_verify_inline_encryption(struct mapped_device *md, struct dm_table *t)
> +{
> + struct blk_keyslot_manager *ksm = dm_construct_keyslot_manager(md, t);
> +
> + if (IS_ERR(ksm))
> + return PTR_ERR(ksm);
> + dm_destroy_keyslot_manager(ksm);
> +
> + return 0;
> +}
> +
> static void dm_update_keyslot_manager(struct request_queue *q,
> struct blk_keyslot_manager *ksm)
> {
There shouldn't be any need to bolt on ksm verification in terms of a
temporary ksm. If you run with my suggestions I just provided in review
of patch 3: dm_table_complete()'s setup of the ksm should also
implicitly validate it.
So this patch, and extra dm_verify_inline_encryption() interface,
shouldn't be needed.
Mike
