diff mbox series

[1/3] block: keyslot-manager: Introduce passthrough keyslot manager

Message ID 20200909234422.76194-2-satyat@google.com (mailing list archive)
State New, archived
Headers show
Series add support for inline encryption to device mapper | expand

Commit Message

Satya Tangirala Sept. 9, 2020, 11:44 p.m. UTC
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 <satyat@google.com>
---
 block/keyslot-manager.c         | 41 +++++++++++++++++++++++++++++++++
 include/linux/keyslot-manager.h |  2 ++
 2 files changed, 43 insertions(+)

Comments

Eric Biggers Sept. 22, 2020, 12:27 a.m. UTC | #1
On Wed, Sep 09, 2020 at 11:44:20PM +0000, Satya Tangirala wrote:
> 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.

To be a bit more concrete, FMP (Flash Memory Protector) on Exynos SoCs is an
example of hardware that supports inline encryption without having the concept
of keyslots.  On that hardware, each request takes an actual key, rather than a
keyslot number.  Likewise, some Mediatek SoCs are like this too.

So support for inline encryption without keyslots is something that is useful
for real hardware, in addition to the device-mapper support which is the initial
use case included in this patchset.

> 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 <satyat@google.com>

Generally looks good, feel free to add:

	Reviewed-by: Eric Biggers <ebiggers@google.com>

However, maybe it's worth reconsidering the suggestion I've made previously
(https://lkml.kernel.org/linux-block/20200326062213.GF858@sol.localdomain) of
separating the crypto capabilities from the keyslot manager.  If we did that,
then this case could be handled by a NULL keyslot manager, rather than a special
kind of keyslot manager that doesn't actually do the keyslot management.

I realize that it's a bit tricky because the key eviction callback would still
be needed.  So maybe it's not really better.  Also, previously other people have
seemed to prefer just the keyslot manager, e.g.
https://lkml.kernel.org/linux-block/20200327170047.GA24682@infradead.org.

Does anyone have any new thoughts on this?

Also, a couple minor comments below.

> @@ -353,6 +372,9 @@ void blk_ksm_reprogram_all_keys(struct blk_keyslot_manager *ksm)
>  {
>  	unsigned int slot;
>  
> +	if (WARN_ON(blk_ksm_is_passthrough(ksm)))
> +		return;
> +

I think the above WARN_ON() should just be removed:

	if (blk_ksm_is_passthrough(ksm))
		return;

Otherwise callers might need to conditionally call blk_ksm_reprogram_all_keys()
depending on whether there are keyslots or not.

> +/**
> + * 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 don't have a small fixed
> + * number of keyslots, and for layered devices.
> + *

Maybe:

"inline encryption hardware that don't have a small fixed number of keyslots"

=>

"inline encryption hardware that doesn't have the concept of keyslots"
diff mbox series

Patch

diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c
index 35abcb1ec051..60ac406d54b9 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.
@@ -198,6 +203,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);
@@ -318,6 +327,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)
@@ -353,6 +372,9 @@  void blk_ksm_reprogram_all_keys(struct blk_keyslot_manager *ksm)
 {
 	unsigned int slot;
 
+	if (WARN_ON(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++) {
@@ -394,3 +416,22 @@  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 don't have a small fixed
+ * number of keyslots, and for layered devices.
+ *
+ * See blk_ksm_init() for more details about the parameters.
+ */
+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 */