On Tue, Dec 29 2020 at 3:55am -0500,
Satya Tangirala <satyat@xxxxxxxxxx> wrote:
> Update the device-mapper core to support exposing the inline crypto
> support of the underlying device(s) through the device-mapper device.
>
> This works by creating a "passthrough keyslot manager" for the dm
> device, which declares support for encryption settings which all
> underlying devices support. When a supported setting is used, the bio
> cloning code handles cloning the crypto context to the bios for all the
> underlying devices. When an unsupported setting is used, the blk-crypto
> fallback is used as usual.
>
> Crypto support on each underlying device is ignored unless the
> corresponding dm target opts into exposing it. This is needed because
> for inline crypto to semantically operate on the original bio, the data
> must not be transformed by the dm target. Thus, targets like dm-linear
> can expose crypto support of the underlying device, but targets like
> dm-crypt can't. (dm-crypt could use inline crypto itself, though.)
>
> A DM device's table can only be changed if the "new" inline encryption
> capabilities are a (*not* necessarily strict) superset of the "old" inline
> encryption capabilities. Attempts to make changes to the table that result
> in some inline encryption capability becoming no longer supported will be
> rejected.
>
> For the sake of clarity, key eviction from underlying devices will be
> handled in a future patch.
>
> Co-developed-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> Signed-off-by: Satya Tangirala <satyat@xxxxxxxxxx>
> ---
> drivers/md/dm.c | 164 +++++++++++++++++++++++++++++++-
> include/linux/device-mapper.h | 6 ++
> include/linux/keyslot-manager.h | 8 ++
> 3 files changed, 177 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index b3c3c8b4cb42..13b9c8e2e21b 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -28,6 +28,7 @@
> #include <linux/refcount.h>
> #include <linux/part_stat.h>
> #include <linux/blk-crypto.h>
> +#include <linux/keyslot-manager.h>
>
> #define DM_MSG_PREFIX "core"
>
> @@ -1718,6 +1719,8 @@ static const struct dax_operations dm_dax_ops;
>
> static void dm_wq_work(struct work_struct *work);
>
> +static void dm_destroy_inline_encryption(struct request_queue *q);
> +
> static void cleanup_mapped_device(struct mapped_device *md)
> {
> if (md->wq)
> @@ -1739,8 +1742,10 @@ static void cleanup_mapped_device(struct mapped_device *md)
> put_disk(md->disk);
> }
>
> - if (md->queue)
> + if (md->queue) {
> + dm_destroy_inline_encryption(md->queue);
> blk_cleanup_queue(md->queue);
> + }
>
> cleanup_srcu_struct(&md->io_barrier);
>
> @@ -1937,6 +1942,150 @@ static void event_callback(void *context)
> dm_issue_global_event();
> }
>
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +
> +struct dm_keyslot_manager {
> + struct blk_keyslot_manager ksm;
> + struct mapped_device *md;
> +};
> +
> +static int device_intersect_crypto_modes(struct dm_target *ti,
> + struct dm_dev *dev, sector_t start,
> + sector_t len, void *data)
> +{
> + struct blk_keyslot_manager *parent = data;
> + struct blk_keyslot_manager *child = bdev_get_queue(dev->bdev)->ksm;
> +
> + blk_ksm_intersect_modes(parent, child);
> + return 0;
> +}
> +
> +static void dm_destroy_keyslot_manager(struct blk_keyslot_manager *ksm)
> +{
> + struct dm_keyslot_manager *dksm = container_of(ksm,
> + struct dm_keyslot_manager,
> + ksm);
> +
> + if (!ksm)
> + return;
> +
> + blk_ksm_destroy(ksm);
> + kfree(dksm);
> +}
> +
> +/*
> + * Constructs and returns a keyslot manager that represents the crypto
> + * capabilities of the devices described by the dm_table. However, if the
> + * constructed keyslot manager does not support a superset of the crypto
> + * capabilities supported by the current keyslot manager of the mapped_device,
> + * it returns an error instead, since we don't support restricting crypto
> + * capabilities on table changes. Finally, if the constructed keyslot manager
> + * doesn't actually support any crypto modes at all, it just returns NULL.
> + */
> +static struct blk_keyslot_manager *
> +dm_construct_keyslot_manager(struct mapped_device *md, struct dm_table *t)
> +{
> + struct dm_keyslot_manager *dksm;
> + struct blk_keyslot_manager *ksm;
> + struct dm_target *ti;
> + unsigned int i;
> + bool ksm_is_empty = true;
> +
> + dksm = kmalloc(sizeof(*dksm), GFP_KERNEL);
> + if (!dksm)
> + return ERR_PTR(-ENOMEM);
> + dksm->md = md;
> +
> + ksm = &dksm->ksm;
> + blk_ksm_init_passthrough(ksm);
> + ksm->max_dun_bytes_supported = UINT_MAX;
> + memset(ksm->crypto_modes_supported, 0xFF,
> + sizeof(ksm->crypto_modes_supported));
> +
> + for (i = 0; i < dm_table_get_num_targets(t); i++) {
> + ti = dm_table_get_target(t, i);
> +
> + if (!dm_target_passes_crypto(ti->type)) {
> + blk_ksm_intersect_modes(ksm, NULL);
> + break;
> + }
> + if (!ti->type->iterate_devices)
> + continue;
> + ti->type->iterate_devices(ti, device_intersect_crypto_modes,
> + ksm);
> + }
> +
> + if (!blk_ksm_is_superset(ksm, md->queue->ksm)) {
> + DMWARN("Inline encryption capabilities of new DM table were more restrictive than the old table's. This is not supported!");
> + dm_destroy_keyslot_manager(ksm);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + /*
> + * If the new KSM doesn't actually support any crypto modes, we may as
> + * well represent it with a NULL ksm.
> + */
> + ksm_is_empty = true;
> + for (i = 0; i < ARRAY_SIZE(ksm->crypto_modes_supported); i++) {
> + if (ksm->crypto_modes_supported[i]) {
> + ksm_is_empty = false;
> + break;
> + }
> + }
> +
> + if (ksm_is_empty) {
> + dm_destroy_keyslot_manager(ksm);
> + ksm = NULL;
> + }
> +
> + return ksm;
> +}
> +
> +static void dm_update_keyslot_manager(struct request_queue *q,
> + struct blk_keyslot_manager *ksm)
> +{
> + if (!ksm)
> + return;
> +
> + /* Make the ksm less restrictive */
> + if (!q->ksm) {
> + blk_ksm_register(ksm, q);
> + } else {
> + blk_ksm_update_capabilities(q->ksm, ksm);
> + dm_destroy_keyslot_manager(ksm);
> + }
> +}
> +
> +static void dm_destroy_inline_encryption(struct request_queue *q)
> +{
> + if (!q->ksm)
> + return;
> + dm_destroy_keyslot_manager(q->ksm);
> +}
> +
> +#else /* CONFIG_BLK_INLINE_ENCRYPTION */
> +
> +static inline struct blk_keyslot_manager *
> +dm_construct_keyslot_manager(struct mapped_device *md, struct dm_table *t)
> +{
> + return NULL;
> +}
> +
> +static void dm_update_keyslot_manager(struct request_queue *q,
> + struct blk_keyslot_manager *ksm)
> +{
> +}
> +
> +static void dm_destroy_keyslot_manager(struct blk_keyslot_manager *ksm)
> +{
> +}
> +
> +static inline void dm_destroy_inline_encryption(struct request_queue *q)
> +{
> +}
> +
> +#endif /* !CONFIG_BLK_INLINE_ENCRYPTION */
> +
> /*
> * Returns old map, which caller must destroy.
> */
> @@ -2332,6 +2481,7 @@ struct dm_table *dm_swap_table(struct mapped_device *md, struct dm_table *table)
> {
> struct dm_table *live_map = NULL, *map = ERR_PTR(-EINVAL);
> struct queue_limits limits;
> + struct blk_keyslot_manager *ksm;
> int r;
>
> mutex_lock(&md->suspend_lock);
> @@ -2361,7 +2511,19 @@ struct dm_table *dm_swap_table(struct mapped_device *md, struct dm_table *table)
> }
> }
>
> + ksm = dm_construct_keyslot_manager(md, table);
> + if (IS_ERR(ksm)) {
> + map = ERR_CAST(ksm);
> + goto out;
> + }
> +
The work done in dm_construct_keyslot_manager() is inappropriate to do
at this point in the DM core code.
This should be split into 2 phases:
1) allocation/setup of ksm in dm-table.c:dm_table_complete()
2) quick install of ksm in dm.c:__bind()
Memory allocation should be done as part of dm_table_complete().
Meaning I think the ksm should be allocated on demand as part of
dm_table creation. Then if in dm_swap_table()'s __bind() table->ksm
isn't NULL, dm_update_keyslot_manager() is called.
dm_update_keyslot_manager() should be updated to take dm_table rather
than ksm (and table->ksm is set to NULL at end).
But if DM table creation's validation of ksm fails, then any newly
allocated ksm (stored in table->ksm) should be destroyed as part of
dm_table_destroy(). This should also eliminate the need for an
intermediate 'struct dm_keyslot_manager'
Mike
> map = __bind(md, table, &limits);
> +
> + if (IS_ERR(map))
> + dm_destroy_keyslot_manager(ksm);
> + else
> + dm_update_keyslot_manager(md->queue, ksm);
> +
> dm_issue_global_event();
>
> out:
> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index 61a66fb8ebb3..0d5794a0a89a 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -257,6 +257,12 @@ struct target_type {
> #define DM_TARGET_NOWAIT 0x00000080
> #define dm_target_supports_nowait(type) ((type)->features & DM_TARGET_NOWAIT)
>
> +/*
> + *
> + */
> +#define DM_TARGET_PASSES_CRYPTO 0x00000100
> +#define dm_target_passes_crypto(type) ((type)->features & DM_TARGET_PASSES_CRYPTO)
> +
> struct dm_target {
> struct dm_table *table;
> struct target_type *type;
> diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h
> index 164568f52be7..9164c1c72288 100644
> --- a/include/linux/keyslot-manager.h
> +++ b/include/linux/keyslot-manager.h
> @@ -11,6 +11,8 @@
>
> struct blk_keyslot_manager;
>
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +
> /**
> * struct blk_ksm_ll_ops - functions to manage keyslots in hardware
> * @keyslot_program: Program the specified key into the specified slot in the
> @@ -114,4 +116,10 @@ bool blk_ksm_is_superset(struct blk_keyslot_manager *ksm_superset,
> void blk_ksm_update_capabilities(struct blk_keyslot_manager *target_ksm,
> struct blk_keyslot_manager *reference_ksm);
>
> +#else /* CONFIG_BLK_INLINE_ENCRYPTION */
> +
> +static inline void blk_ksm_destroy(struct blk_keyslot_manager *ksm) { }
> +
> +#endif /* CONFIG_BLK_INLINE_ENCRYPTION */
> +
> #endif /* __LINUX_KEYSLOT_MANAGER_H */
> --
> 2.29.2.729.g45daf8777d-goog
>
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
