On Tue, Dec 29 2020 at 3:55am -0500,
Satya Tangirala <satyat@xxxxxxxxxx> 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 <satyat@xxxxxxxxxx>
> ---
> 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
>
