Re: [PATCH v6 6/9] scsi: ufs: Add inline encryption support to UFS

[Eric Biggers <ebiggers@kernel.org>]

On Wed, Dec 18, 2019 at 06:51:33AM -0800, Satya Tangirala wrote:
> Wire up ufshcd.c with the UFS Crypto API, the block layer inline
> encryption additions and the keyslot manager.
> 
> Signed-off-by: Satya Tangirala <satyat@google.com>
> ---
>  drivers/scsi/ufs/ufshcd-crypto.c | 30 ++++++++++++++++++
>  drivers/scsi/ufs/ufshcd-crypto.h | 21 +++++++++++++
>  drivers/scsi/ufs/ufshcd.c        | 54 +++++++++++++++++++++++++++++---
>  drivers/scsi/ufs/ufshcd.h        |  8 +++++
>  4 files changed, 108 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd-crypto.c b/drivers/scsi/ufs/ufshcd-crypto.c
> index b0aa072d9009..749c325686a7 100644
> --- a/drivers/scsi/ufs/ufshcd-crypto.c
> +++ b/drivers/scsi/ufs/ufshcd-crypto.c
> @@ -352,6 +352,36 @@ void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
>  }
>  EXPORT_SYMBOL_GPL(ufshcd_crypto_setup_rq_keyslot_manager);
>  
> +int ufshcd_prepare_lrbp_crypto(struct ufs_hba *hba,
> +			       struct scsi_cmnd *cmd,
> +			       struct ufshcd_lrb *lrbp)
> +{
> +	struct bio_crypt_ctx *bc;
> +
> +	if (!bio_crypt_should_process(cmd->request)) {
> +		lrbp->crypto_enable = false;
> +		return 0;
> +	}
> +	bc = cmd->request->bio->bi_crypt_context;
> +
> +	if (WARN_ON(!ufshcd_is_crypto_enabled(hba))) {
> +		/*
> +		 * Upper layer asked us to do inline encryption
> +		 * but that isn't enabled, so we fail this request.
> +		 */
> +		return -EINVAL;
> +	}
> +	if (!ufshcd_keyslot_valid(hba, bc->bc_keyslot))
> +		return -EINVAL;
> +
> +	lrbp->crypto_enable = true;
> +	lrbp->crypto_key_slot = bc->bc_keyslot;
> +	lrbp->data_unit_num = bc->bc_dun[0];
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_prepare_lrbp_crypto);

The UFS driver only uses the first 64 bits of the DUN, but in this version of
the patchset the DUN in the bio_crypt_ctx can be up to the real length of the
algorithm's IV -- which for AES-256-XTS is 128 bits.  So if the user were to
specify anything nonzero in bits 64-127, the crypto would be done incorrectly.

(This case isn't encountered with fscrypt.  But it's still an issue with the
overall approach.)

So there needs to be a way for drivers to declare the max_dun_size they support,
and prevent them from being used with longer DUNs.

- Eric