2020-12-29 08:58:25

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v3 0/6] add support for inline encryption to device mapper

This patch series adds support for inline encryption to the device mapper.

Patch 1 introduces the "passthrough" keyslot manager.

The regular keyslot manager is designed for inline encryption hardware that
have only a small fixed number of keyslots. A DM device itself does not
actually have only a small fixed number of keyslots - it doesn't actually
have any keyslots in the first place, and programming an encryption context
into a DM device doesn't make much semantic sense. It is possible for a DM
device to set up a keyslot manager with some "sufficiently large" number of
keyslots in its request queue, so that upper layers can use the inline
encryption capabilities of the DM device's underlying devices, but the
memory being allocated for the DM device's keyslots is a waste since they
won't actually be used by the DM device.

The passthrough keyslot manager solves this issue - when the block layer
sees that a request queue has a passthrough keyslot manager, it doesn't
attempt to program any encryption context into the keyslot manager. The
passthrough keyslot manager only allows the device to expose its inline
encryption capabilities, and a way for upper layers to evict keys if
necessary.

There also exist inline encryption hardware that can handle encryption
contexts directly, and allow users to pass them a data request along with
the encryption context (as opposed to inline encryption hardware that
require users to first program a keyslot with an encryption context, and
then require the users to pass the keyslot index with the data request).
Such devices can also make use of the passthrough keyslot manager.

Patch 2 introduces some keyslot manager functions useful for the device
mapper.

Patch 3 introduces the changes for inline encryption support for the device
mapper. A DM device only exposes the intersection of the crypto
capabilities of its underlying devices. This is so that in case a bio with
an encryption context is eventually mapped to an underlying device that
doesn't support that encryption context, the blk-crypto-fallback's cipher
tfms are allocated ahead of time by the call to blk_crypto_start_using_key.

Each DM target can now also specify the "DM_TARGET_PASSES_CRYPTO" flag in
the target type features to opt-in to supporting passing through the
underlying inline encryption capabilities. This flag is needed because it
doesn't make much semantic sense for certain targets like dm-crypt to
expose the underlying inline encryption capabilities to the upper layers.
Again, the DM exposes inline encryption capabilities of the underlying
devices only if all of them opt-in to passing through inline encryption
support.

A DM device's keyslot manager is set up whenever a new table is swapped in.
This patch only allows the keyslot manager's capabilities to *expand*
because of table changes. In this patch, the new inline encryption
capabilities are only verified and used when the table is *swapped* -
nothing is done when a new table is loaded (Patch 5 changes this
behaviour).

This patch also only exposes the intersection of the underlying device's
capabilities, which has the effect of causing en/decryption of a bio to
fall back to the kernel crypto API (if the fallback is enabled) whenever
any of the underlying devices doesn't support the encryption context of the
bio - it might be possible to make the bio only fall back to the kernel
crypto API if the bio's target underlying device doesn't support the bio's
encryption context, but the use case may be uncommon enough in the first
place not to warrant worrying about it right now.

Patch 4 makes DM evict a key from all its underlying devices when asked to
evict a key.

Patch 5 verifies the inline encryption capabilities of a new table when
it's loaded. Any attempts to load a new table that would cause crypto
capabilities to be dropped are rejected at load time, with this patch.
Still, the keyslot manager for the DM device is only modified when the
table is actually swapped in, since the capabilities of the device may
change further due to changes in capabilities of underlying devices between
the time the table load and table swap happen.

Patch 6 makes some DM targets opt-in to passing through inline encryption
support. It does not (yet) try to enable this option with dm-raid, since
users can "hot add" disks to a raid device, which makes this not completely
straightforward (we'll need to ensure that any "hot added" disks must have
a superset of the inline encryption capabilities of the rest of the disks
in the raid device, due to the way Patch 2 of this series works).

Changes v2 => v3:
- Split up the main DM patch into 4 separate patches
- Removed the priv variable added to struct keyslot manager in v2
- Use a flag in target type features for opting-in to inline encryption
support, instead of using "may_passthrough_inline_crypto"
- cleanups and restructure code

Changes v1 => v2:
- Introduce private field to struct blk_keyslot_manager
- Allow the DM keyslot manager to expand its crypto capabilities if the
table is changed.
- Make DM reject table changes that would otherwise cause crypto
capabilities to be dropped.
- Allocate the DM device's keyslot manager only when at least one crypto
capability is supported (since a NULL value for q->ksm represents "no
crypto support" anyway).
- Remove the struct blk_keyslot_manager field from struct mapped_device.
This patch now relies on just directly setting up the keyslot manager in
the request queue, since each DM device is tied to only 1 queue.

Satya Tangirala (6):
block: keyslot-manager: Introduce passthrough keyslot manager
block: keyslot-manager: Introduce functions for device mapper support
dm: add support for passing through inline crypto support
dm: Support key eviction from keyslot managers of underlying devices
dm: Verify inline encryption capabilities of new table when it is
loaded
dm: set DM_TARGET_PASSES_CRYPTO feature for some targets

block/blk-crypto.c | 1 +
block/keyslot-manager.c | 130 +++++++++++++++++
drivers/md/dm-flakey.c | 4 +-
drivers/md/dm-ioctl.c | 8 ++
drivers/md/dm-linear.c | 5 +-
drivers/md/dm.c | 242 +++++++++++++++++++++++++++++++-
drivers/md/dm.h | 19 +++
include/linux/device-mapper.h | 6 +
include/linux/keyslot-manager.h | 19 +++
9 files changed, 430 insertions(+), 4 deletions(-)

--
2.29.2.729.g45daf8777d-goog


2020-12-29 08:58:34

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v3 1/6] block: keyslot-manager: Introduce passthrough keyslot manager

The device mapper may map over devices that have inline encryption
capabilities, and to make use of those capabilities, the DM device must
itself advertise those inline encryption capabilities. One way to do this
would be to have the DM device set up a keyslot manager with a
"sufficiently large" number of keyslots, but that would use a lot of
memory. Also, the DM device itself has no "keyslots", and it doesn't make
much sense to talk about "programming a key into a DM device's keyslot
manager", so all that extra memory used to represent those keyslots is just
wasted. All a DM device really needs to be able to do is advertise the
crypto capabilities of the underlying devices in a coherent manner and
expose a way to evict keys from the underlying devices.

There are also devices with inline encryption hardware that do not
have a limited number of keyslots. One can send a raw encryption key along
with a bio to these devices (as opposed to typical inline encryption
hardware that require users to first program a raw encryption key into a
keyslot, and send the index of that keyslot along with the bio). These
devices also only need the same things from the keyslot manager that DM
devices need - a way to advertise crypto capabilities and potentially a way
to expose a function to evict keys from hardware.

So we introduce a "passthrough" keyslot manager that provides a way to
represent a keyslot manager that doesn't have just a limited number of
keyslots, and for which do not require keys to be programmed into keyslots.
DM devices can set up a passthrough keyslot manager in their request
queues, and advertise appropriate crypto capabilities based on those of the
underlying devices. Blk-crypto does not attempt to program keys into any
keyslots in the passthrough keyslot manager. Instead, if/when the bio is
resubmitted to the underlying device, blk-crypto will try to program the
key into the underlying device's keyslot manager.

Signed-off-by: Satya Tangirala <[email protected]>
Reviewed-by: Eric Biggers <[email protected]>
---
block/keyslot-manager.c | 39 +++++++++++++++++++++++++++++++++
include/linux/keyslot-manager.h | 2 ++
2 files changed, 41 insertions(+)

diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c
index 86f8195d8039..ac7ce83a76e8 100644
--- a/block/keyslot-manager.c
+++ b/block/keyslot-manager.c
@@ -62,6 +62,11 @@ static inline void blk_ksm_hw_exit(struct blk_keyslot_manager *ksm)
pm_runtime_put_sync(ksm->dev);
}

+static inline bool blk_ksm_is_passthrough(struct blk_keyslot_manager *ksm)
+{
+ return ksm->num_slots == 0;
+}
+
/**
* blk_ksm_init() - Initialize a keyslot manager
* @ksm: The keyslot_manager to initialize.
@@ -205,6 +210,10 @@ blk_status_t blk_ksm_get_slot_for_key(struct blk_keyslot_manager *ksm,
int err;

*slot_ptr = NULL;
+
+ if (blk_ksm_is_passthrough(ksm))
+ return BLK_STS_OK;
+
down_read(&ksm->lock);
slot = blk_ksm_find_and_grab_keyslot(ksm, key);
up_read(&ksm->lock);
@@ -325,6 +334,16 @@ int blk_ksm_evict_key(struct blk_keyslot_manager *ksm,
struct blk_ksm_keyslot *slot;
int err = 0;

+ if (blk_ksm_is_passthrough(ksm)) {
+ if (ksm->ksm_ll_ops.keyslot_evict) {
+ blk_ksm_hw_enter(ksm);
+ err = ksm->ksm_ll_ops.keyslot_evict(ksm, key, -1);
+ blk_ksm_hw_exit(ksm);
+ return err;
+ }
+ return 0;
+ }
+
blk_ksm_hw_enter(ksm);
slot = blk_ksm_find_keyslot(ksm, key);
if (!slot)
@@ -360,6 +379,9 @@ void blk_ksm_reprogram_all_keys(struct blk_keyslot_manager *ksm)
{
unsigned int slot;

+ if (blk_ksm_is_passthrough(ksm))
+ return;
+
/* This is for device initialization, so don't resume the device */
down_write(&ksm->lock);
for (slot = 0; slot < ksm->num_slots; slot++) {
@@ -401,3 +423,20 @@ void blk_ksm_unregister(struct request_queue *q)
{
q->ksm = NULL;
}
+
+/**
+ * blk_ksm_init_passthrough() - Init a passthrough keyslot manager
+ * @ksm: The keyslot manager to init
+ *
+ * Initialize a passthrough keyslot manager.
+ * Called by e.g. storage drivers to set up a keyslot manager in their
+ * request_queue, when the storage driver wants to manage its keys by itself.
+ * This is useful for inline encryption hardware that doesn't have the concept
+ * of keyslots, and for layered devices.
+ */
+void blk_ksm_init_passthrough(struct blk_keyslot_manager *ksm)
+{
+ memset(ksm, 0, sizeof(*ksm));
+ init_rwsem(&ksm->lock);
+}
+EXPORT_SYMBOL_GPL(blk_ksm_init_passthrough);
diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h
index 18f3f5346843..323e15dd6fa7 100644
--- a/include/linux/keyslot-manager.h
+++ b/include/linux/keyslot-manager.h
@@ -103,4 +103,6 @@ void blk_ksm_reprogram_all_keys(struct blk_keyslot_manager *ksm);

void blk_ksm_destroy(struct blk_keyslot_manager *ksm);

+void blk_ksm_init_passthrough(struct blk_keyslot_manager *ksm);
+
#endif /* __LINUX_KEYSLOT_MANAGER_H */
--
2.29.2.729.g45daf8777d-goog

2020-12-29 08:58:36

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v3 3/6] dm: add support for passing through inline crypto support

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 <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
Signed-off-by: Satya Tangirala <[email protected]>
---
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;
+ }
+
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

2020-12-29 08:58:54

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v3 5/6] dm: Verify inline encryption capabilities of new table when it is loaded

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 <[email protected]>
---
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)
{
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index fffe1e289c53..eaf92e4cbe70 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -208,4 +208,23 @@ void dm_free_md_mempools(struct dm_md_mempools *pools);
*/
unsigned dm_get_reserved_bio_based_ios(void);

+/*
+ * Inline Encryption
+ */
+struct blk_keyslot_manager;
+
+#ifdef CONFIG_BLK_INLINE_ENCRYPTION
+
+int dm_verify_inline_encryption(struct mapped_device *md, struct dm_table *t);
+
+#else /* !CONFIG_BLK_INLINE_ENCRYPTION */
+
+static inline int dm_verify_inline_encryption(struct mapped_device *md,
+ struct dm_table *t)
+{
+ return 0;
+}
+
+#endif /* !CONFIG_BLK_INLINE_ENCRYPTION */
+
#endif
--
2.29.2.729.g45daf8777d-goog

2020-12-29 08:59:51

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v3 6/6] dm: set DM_TARGET_PASSES_CRYPTO feature for some targets

dm-linear and dm-flakey obviously can pass through inline crypto support.

Co-developed-by: Eric Biggers <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
Signed-off-by: Satya Tangirala <[email protected]>
---
drivers/md/dm-flakey.c | 4 +++-
drivers/md/dm-linear.c | 5 +++--
2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c
index a2cc9e45cbba..30c6bc151213 100644
--- a/drivers/md/dm-flakey.c
+++ b/drivers/md/dm-flakey.c
@@ -482,8 +482,10 @@ static struct target_type flakey_target = {
.name = "flakey",
.version = {1, 5, 0},
#ifdef CONFIG_BLK_DEV_ZONED
- .features = DM_TARGET_ZONED_HM,
+ .features = DM_TARGET_ZONED_HM | DM_TARGET_PASSES_CRYPTO,
.report_zones = flakey_report_zones,
+#else
+ .features = DM_TARGET_PASSES_CRYPTO,
#endif
.module = THIS_MODULE,
.ctr = flakey_ctr,
diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 00774b5d7668..fc9c4272c10d 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -229,10 +229,11 @@ static struct target_type linear_target = {
.version = {1, 4, 0},
#ifdef CONFIG_BLK_DEV_ZONED
.features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT |
- DM_TARGET_ZONED_HM,
+ DM_TARGET_ZONED_HM | DM_TARGET_PASSES_CRYPTO,
.report_zones = linear_report_zones,
#else
- .features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT,
+ .features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT |
+ DM_TARGET_PASSES_CRYPTO,
#endif
.module = THIS_MODULE,
.ctr = linear_ctr,
--
2.29.2.729.g45daf8777d-goog

2020-12-29 09:00:10

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v3 2/6] block: keyslot-manager: Introduce functions for device mapper support

Introduce blk_ksm_update_capabilities() to update the capabilities of
a keyslot manager (ksm) in-place. The pointer to a ksm in a device's
request queue may not be easily replaced, because upper layers like
the filesystem might access it (e.g. for programming keys/checking
capabilities) at the same time the device wants to replace that
request queue's ksm (and free the old ksm's memory). This function
allows the device to update the capabilities of the ksm in its request
queue directly.

Also introduce blk_ksm_is_superset() which checks whether one ksm's
capabilities are a (not necessarily strict) superset of another ksm's.
The blk-crypto framework requires that crypto capabilities that were
advertised when a bio was created continue to be supported by the
device until that bio is ended - in practice this probably means that
a device's advertised crypto capabilities can *never* "shrink" (since
there's no synchronization between bio creation and when a device may
want to change its advertised capabilities) - so a previously
advertised crypto capability must always continue to be supported.
This function can be used to check that a new ksm is a valid
replacement for an old ksm.

Signed-off-by: Satya Tangirala <[email protected]>
---
block/keyslot-manager.c | 91 +++++++++++++++++++++++++++++++++
include/linux/keyslot-manager.h | 9 ++++
2 files changed, 100 insertions(+)

diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c
index ac7ce83a76e8..f13ab7410eca 100644
--- a/block/keyslot-manager.c
+++ b/block/keyslot-manager.c
@@ -424,6 +424,97 @@ void blk_ksm_unregister(struct request_queue *q)
q->ksm = NULL;
}

+/**
+ * blk_ksm_intersect_modes() - restrict supported modes by child device
+ * @parent: The keyslot manager for parent device
+ * @child: The keyslot manager for child device, or NULL
+ *
+ * Clear any crypto mode support bits in @parent that aren't set in @child.
+ * If @child is NULL, then all parent bits are cleared.
+ *
+ * Only use this when setting up the keyslot manager for a layered device,
+ * before it's been exposed yet.
+ */
+void blk_ksm_intersect_modes(struct blk_keyslot_manager *parent,
+ const struct blk_keyslot_manager *child)
+{
+ if (child) {
+ unsigned int i;
+
+ parent->max_dun_bytes_supported =
+ min(parent->max_dun_bytes_supported,
+ child->max_dun_bytes_supported);
+ for (i = 0; i < ARRAY_SIZE(child->crypto_modes_supported);
+ i++) {
+ parent->crypto_modes_supported[i] &=
+ child->crypto_modes_supported[i];
+ }
+ } else {
+ parent->max_dun_bytes_supported = 0;
+ memset(parent->crypto_modes_supported, 0,
+ sizeof(parent->crypto_modes_supported));
+ }
+}
+EXPORT_SYMBOL_GPL(blk_ksm_intersect_modes);
+
+/**
+ * blk_ksm_is_superset() - Check if a KSM supports a superset of crypto modes
+ * and DUN bytes that another KSM supports. Here,
+ * "superset" refers to the mathematical meaning of the
+ * word - i.e. if two KSMs have the *same* capabilities,
+ * they *are* considered supersets of each other.
+ * @ksm_superset: The KSM that we want to verify is a superset
+ * @ksm_subset: The KSM that we want to verify is a subset
+ *
+ * Return: True if @ksm_superset supports a superset of the crypto modes and DUN
+ * bytes that @ksm_subset supports.
+ */
+bool blk_ksm_is_superset(struct blk_keyslot_manager *ksm_superset,
+ struct blk_keyslot_manager *ksm_subset)
+{
+ int i;
+
+ if (!ksm_subset)
+ return true;
+
+ if (!ksm_superset)
+ return false;
+
+ for (i = 0; i < ARRAY_SIZE(ksm_superset->crypto_modes_supported); i++) {
+ if (ksm_subset->crypto_modes_supported[i] &
+ (~ksm_superset->crypto_modes_supported[i])) {
+ return false;
+ }
+ }
+
+ if (ksm_subset->max_dun_bytes_supported >
+ ksm_superset->max_dun_bytes_supported) {
+ return false;
+ }
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(blk_ksm_is_superset);
+
+/**
+ * blk_ksm_update_capabilities() - Update the restrictions of a KSM to those of
+ * another KSM
+ * @target_ksm: The KSM whose restrictions to update.
+ * @reference_ksm: The KSM to whose restrictions this function will update
+ * @target_ksm's restrictions to,
+ */
+void blk_ksm_update_capabilities(struct blk_keyslot_manager *target_ksm,
+ struct blk_keyslot_manager *reference_ksm)
+{
+ memcpy(target_ksm->crypto_modes_supported,
+ reference_ksm->crypto_modes_supported,
+ sizeof(target_ksm->crypto_modes_supported));
+
+ target_ksm->max_dun_bytes_supported =
+ reference_ksm->max_dun_bytes_supported;
+}
+EXPORT_SYMBOL_GPL(blk_ksm_update_capabilities);
+
/**
* blk_ksm_init_passthrough() - Init a passthrough keyslot manager
* @ksm: The keyslot manager to init
diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h
index 323e15dd6fa7..164568f52be7 100644
--- a/include/linux/keyslot-manager.h
+++ b/include/linux/keyslot-manager.h
@@ -103,6 +103,15 @@ void blk_ksm_reprogram_all_keys(struct blk_keyslot_manager *ksm);

void blk_ksm_destroy(struct blk_keyslot_manager *ksm);

+void blk_ksm_intersect_modes(struct blk_keyslot_manager *parent,
+ const struct blk_keyslot_manager *child);
+
void blk_ksm_init_passthrough(struct blk_keyslot_manager *ksm);

+bool blk_ksm_is_superset(struct blk_keyslot_manager *ksm_superset,
+ struct blk_keyslot_manager *ksm_subset);
+
+void blk_ksm_update_capabilities(struct blk_keyslot_manager *target_ksm,
+ struct blk_keyslot_manager *reference_ksm);
+
#endif /* __LINUX_KEYSLOT_MANAGER_H */
--
2.29.2.729.g45daf8777d-goog

2021-01-14 16:10:49

by Mike Snitzer

[permalink] [raw]
Subject: Re: [PATCH v3 2/6] block: keyslot-manager: Introduce functions for device mapper support

On Tue, Dec 29 2020 at 3:55am -0500,
Satya Tangirala <[email protected]> wrote:

> Introduce blk_ksm_update_capabilities() to update the capabilities of
> a keyslot manager (ksm) in-place. The pointer to a ksm in a device's
> request queue may not be easily replaced, because upper layers like
> the filesystem might access it (e.g. for programming keys/checking
> capabilities) at the same time the device wants to replace that
> request queue's ksm (and free the old ksm's memory). This function
> allows the device to update the capabilities of the ksm in its request
> queue directly.
>
> Also introduce blk_ksm_is_superset() which checks whether one ksm's
> capabilities are a (not necessarily strict) superset of another ksm's.
> The blk-crypto framework requires that crypto capabilities that were
> advertised when a bio was created continue to be supported by the
> device until that bio is ended - in practice this probably means that
> a device's advertised crypto capabilities can *never* "shrink" (since
> there's no synchronization between bio creation and when a device may
> want to change its advertised capabilities) - so a previously
> advertised crypto capability must always continue to be supported.
> This function can be used to check that a new ksm is a valid
> replacement for an old ksm.
>
> Signed-off-by: Satya Tangirala <[email protected]>
> ---
> block/keyslot-manager.c | 91 +++++++++++++++++++++++++++++++++
> include/linux/keyslot-manager.h | 9 ++++
> 2 files changed, 100 insertions(+)
>
> diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c
> index ac7ce83a76e8..f13ab7410eca 100644
> --- a/block/keyslot-manager.c
> +++ b/block/keyslot-manager.c
> @@ -424,6 +424,97 @@ void blk_ksm_unregister(struct request_queue *q)
> q->ksm = NULL;
> }
>
> +/**
> + * blk_ksm_intersect_modes() - restrict supported modes by child device
> + * @parent: The keyslot manager for parent device
> + * @child: The keyslot manager for child device, or NULL
> + *
> + * Clear any crypto mode support bits in @parent that aren't set in @child.
> + * If @child is NULL, then all parent bits are cleared.
> + *
> + * Only use this when setting up the keyslot manager for a layered device,
> + * before it's been exposed yet.
> + */
> +void blk_ksm_intersect_modes(struct blk_keyslot_manager *parent,
> + const struct blk_keyslot_manager *child)
> +{
> + if (child) {
> + unsigned int i;
> +
> + parent->max_dun_bytes_supported =
> + min(parent->max_dun_bytes_supported,
> + child->max_dun_bytes_supported);
> + for (i = 0; i < ARRAY_SIZE(child->crypto_modes_supported);
> + i++) {
> + parent->crypto_modes_supported[i] &=
> + child->crypto_modes_supported[i];
> + }
> + } else {
> + parent->max_dun_bytes_supported = 0;
> + memset(parent->crypto_modes_supported, 0,
> + sizeof(parent->crypto_modes_supported));
> + }
> +}
> +EXPORT_SYMBOL_GPL(blk_ksm_intersect_modes);
> +
> +/**
> + * blk_ksm_is_superset() - Check if a KSM supports a superset of crypto modes
> + * and DUN bytes that another KSM supports. Here,
> + * "superset" refers to the mathematical meaning of the
> + * word - i.e. if two KSMs have the *same* capabilities,
> + * they *are* considered supersets of each other.
> + * @ksm_superset: The KSM that we want to verify is a superset
> + * @ksm_subset: The KSM that we want to verify is a subset
> + *
> + * Return: True if @ksm_superset supports a superset of the crypto modes and DUN
> + * bytes that @ksm_subset supports.
> + */
> +bool blk_ksm_is_superset(struct blk_keyslot_manager *ksm_superset,
> + struct blk_keyslot_manager *ksm_subset)
> +{
> + int i;
> +
> + if (!ksm_subset)
> + return true;
> +
> + if (!ksm_superset)
> + return false;
> +
> + for (i = 0; i < ARRAY_SIZE(ksm_superset->crypto_modes_supported); i++) {
> + if (ksm_subset->crypto_modes_supported[i] &
> + (~ksm_superset->crypto_modes_supported[i])) {
> + return false;
> + }
> + }
> +
> + if (ksm_subset->max_dun_bytes_supported >
> + ksm_superset->max_dun_bytes_supported) {
> + return false;
> + }
> +
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(blk_ksm_is_superset);
> +
> +/**
> + * blk_ksm_update_capabilities() - Update the restrictions of a KSM to those of
> + * another KSM
> + * @target_ksm: The KSM whose restrictions to update.
> + * @reference_ksm: The KSM to whose restrictions this function will update
> + * @target_ksm's restrictions to,
> + */
> +void blk_ksm_update_capabilities(struct blk_keyslot_manager *target_ksm,
> + struct blk_keyslot_manager *reference_ksm)
> +{
> + memcpy(target_ksm->crypto_modes_supported,
> + reference_ksm->crypto_modes_supported,
> + sizeof(target_ksm->crypto_modes_supported));
> +
> + target_ksm->max_dun_bytes_supported =
> + reference_ksm->max_dun_bytes_supported;
> +}
> +EXPORT_SYMBOL_GPL(blk_ksm_update_capabilities);
> +

Given the patch header's preamble about FS possibly accessing/checking
the existing ksm: without any locking or other coordination how is
blk_ksm_update_capabilities() safe?

Please document any assumptions about the caller (e.g. DM) that enables
blk_ksm_update_capabilities() to be used safely.

Mike

> /**
> * blk_ksm_init_passthrough() - Init a passthrough keyslot manager
> * @ksm: The keyslot manager to init
> diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h
> index 323e15dd6fa7..164568f52be7 100644
> --- a/include/linux/keyslot-manager.h
> +++ b/include/linux/keyslot-manager.h
> @@ -103,6 +103,15 @@ void blk_ksm_reprogram_all_keys(struct blk_keyslot_manager *ksm);
>
> void blk_ksm_destroy(struct blk_keyslot_manager *ksm);
>
> +void blk_ksm_intersect_modes(struct blk_keyslot_manager *parent,
> + const struct blk_keyslot_manager *child);
> +
> void blk_ksm_init_passthrough(struct blk_keyslot_manager *ksm);
>
> +bool blk_ksm_is_superset(struct blk_keyslot_manager *ksm_superset,
> + struct blk_keyslot_manager *ksm_subset);
> +
> +void blk_ksm_update_capabilities(struct blk_keyslot_manager *target_ksm,
> + struct blk_keyslot_manager *reference_ksm);
> +
> #endif /* __LINUX_KEYSLOT_MANAGER_H */
> --
> 2.29.2.729.g45daf8777d-goog
>

2021-01-14 18:06:52

by Mike Snitzer

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] dm: add support for passing through inline crypto support

On Tue, Dec 29 2020 at 3:55am -0500,
Satya Tangirala <[email protected]> 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 <[email protected]>
> Signed-off-by: Eric Biggers <[email protected]>
> Signed-off-by: Satya Tangirala <[email protected]>
> ---
> 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
>

2021-01-14 18:11:58

by Mike Snitzer

[permalink] [raw]
Subject: Re: [PATCH v3 5/6] dm: Verify inline encryption capabilities of new table when it is loaded

On Tue, Dec 29 2020 at 3:55am -0500,
Satya Tangirala <[email protected]> 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 <[email protected]>
> ---
> 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

2021-02-01 05:37:35

by Satya Tangirala

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] dm: add support for passing through inline crypto support

On Thu, Jan 14, 2021 at 01:00:49PM -0500, Mike Snitzer wrote:
> On Tue, Dec 29 2020 at 3:55am -0500,
> Satya Tangirala <[email protected]> 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 <[email protected]>
> > Signed-off-by: Eric Biggers <[email protected]>
> > Signed-off-by: Satya Tangirala <[email protected]>
> > ---
> > 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().
I followed your suggestion for v4 (which I just sent out at
https://lore.kernel.org/dm-devel/[email protected]/)

> This should also eliminate the need for an
> intermediate 'struct dm_keyslot_manager'
>
> Mike
>

I'd have liked to remove the struct dm_keyslot_manager too, but I think
it's still necessary because of the key eviction support introduced in
the next patch. dm_keyslot_evict() needs to know the mapped device given
the ksm. In an earlier version of the series, I tried introducing a
private data field to struct blk_keyslot_manager, but that was objected
to (and this approach was suggested instead).

> > 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
> >
>