Re: [PATCH v8 01/11] block: Keyslot Manager for Inline Encryption
From: Eric Biggers <ebiggers@kernel.org>
Date: 2020-03-15 20:08:16
Also in:
linux-block, linux-f2fs-devel, linux-fscrypt, linux-fsdevel, linux-scsi
On Thu, Mar 12, 2020 at 01:02:43AM -0700, Satya Tangirala wrote:
+/**
+ * blk_ksm_init() - Initialize a keyslot manager
+ * @ksm: The keyslot_manager to initialize.
+ * @dev: Device for runtime power management (NULL if none)
+ * @num_slots: The number of key slots to manage.
+ *
+ * Allocate memory for keyslots and initialize a keyslot manager. Called by
+ * e.g. storage drivers to set up a keyslot manager in their request_queue.
+ *
+ * Return: 0 on success, or else a negative error code.
+ */
+int blk_ksm_init(struct keyslot_manager *ksm, struct device *dev,
+ unsigned int num_slots)
+{
+ unsigned int slot;
+ unsigned int i;
+
+ memset(ksm, 0, sizeof(*ksm));
+
+ if (num_slots == 0)
+ return -EINVAL;
+
+ ksm->slots = kvcalloc(num_slots, sizeof(ksm->slots[0]), GFP_KERNEL);
+ if (!ksm->slots)
+ return -ENOMEM;
+
+ ksm->num_slots = num_slots;
+ ksm->dev = dev;Seems that now that keyslot_manager::dev is an unconditional field and callers initialize other fields of the keyslot_manager, the 'dev' parameter should be removed from blk_ksm_init() and the caller should be responsible for setting 'dev' if they need runtime power management.
+/** + * blk_ksm_get_slot_for_key() - Program a key into a keyslot. + * @ksm: The keyslot manager to program the key into. + * @key: Pointer to the key object to program, including the raw key, crypto + * mode, and data unit size. + * @keyslot: A pointer to return the pointer of the allocated keyslot. + * + * Get a keyslot that's been programmed with the specified key. If one already + * exists, return it with incremented refcount. Otherwise, wait for a keyslot + * to become idle and program it. + * + * Context: Process context. Takes and releases ksm->lock. + * Return: BLK_STATUS_OK on success (and keyslot is set to the pointer of the + * allocated keyslot), and BLK_STATUS_IOERR otherwise (and keyslot is + * set to NULL). + */
There can be other errors besides BLK_STATUS_IOERR returned.
+/**
+ * blk_ksm_put_slot() - Release a reference to a slot
+ * @slot: The keyslot to release the reference of.
+ *
+ * Context: Any context.
+ */
+void blk_ksm_put_slot(struct blk_ksm_keyslot *slot)
+{
+ struct keyslot_manager *ksm = slot->ksm;
+ unsigned long flags;
+
+ if (!slot)
+ return;
+
+ if (atomic_dec_and_lock_irqsave(&slot->slot_refs,
+ &ksm->idle_slots_lock, flags)) {
+ list_add_tail(&slot->idle_slot_node,
+ &ksm->idle_slots);Nit: the arguments to list_add_tail() fit on one line
quoted hunk ↗ jump to hunk
diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h new file mode 100644 index 000000000000..7f88ed02faee --- /dev/null +++ b/include/linux/keyslot-manager.h@@ -0,0 +1,108 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright 2019 Google LLC + */ + +#ifndef __LINUX_KEYSLOT_MANAGER_H +#define __LINUX_KEYSLOT_MANAGER_H + +#include <linux/bio.h> +#include <linux/blk-crypto.h> + +struct keyslot_manager; + +/** + * struct keyslot_mgmt_ll_ops - functions to manage keyslots in hardware + * @keyslot_program: Program the specified key into the specified slot in the + * inline encryption hardware. + * @keyslot_evict: Evict key from the specified keyslot in the hardware. + * The key is provided so that e.g. dm layers can evict + * keys from the devices that they map over. + * Returns 0 on success, -errno otherwise. + * + * This structure should be provided by storage device drivers when they set up + * a keyslot manager - this structure holds the function ptrs that the keyslot + * manager will use to manipulate keyslots in the hardware. + */ +struct keyslot_mgmt_ll_ops { + blk_status_t (*keyslot_program)(struct keyslot_manager *ksm, + const struct blk_crypto_key *key, + unsigned int slot); + int (*keyslot_evict)(struct keyslot_manager *ksm, + const struct blk_crypto_key *key, + unsigned int slot); +};
As I mentioned on one of the UFS patches, I'm not sure it's better to make keyslot_program return blk_status_t rather than errno as it did before. blk_ksm_get_slot_for_key() could just do the translation to blk_status_t.
+struct keyslot_manager {Now that the "blk_" prefix has been added to all the functions, perhaps the actual struct should be renamed to "blk_keyslot_manager" too? And likewise "blk_keyslot_mgmt_ll_ops".