Re: [PATCH v1 06/11] scsi: ufs: core: add slot path to ufshcd_prepare_lrbp_crypto

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Malformed crypto contexts with `data_unit_size_bits == 0` silently bypass encryption instead of failing the request.
--

commit 406714defb60086f0a0900753b849815524e2e23
Author: linlzhan <linlin.zhang@oss.qualcomm.com>

scsi: ufs: core: add slot path to ufshcd_prepare_lrbp_crypto

This commit extends ufshcd_prepare_lrbp_crypto() to support bios that carry a
pre-programmed physical ICE keyslot index in crypt_ctx->bc_slot instead of
a blk_crypto_key pointer. It adds a check for bc_slot.data_unit_size_bits to
determine if the slot path should be used.

> diff --git a/drivers/ufs/core/ufshcd-crypto.h b/drivers/ufs/core/ufshcd-crypto.h
> index 8f66db94e179a..2fc5601c0f769 100644
> --- a/drivers/ufs/core/ufshcd-crypto.h
> +++ b/drivers/ufs/core/ufshcd-crypto.h
> @@ -15,13 +15,21 @@
>  static inline void ufshcd_prepare_lrbp_crypto(struct request *rq,
>  					      struct ufshcd_lrb *lrbp)
>  {
> -	if (!rq || !rq->crypt_keyslot) {
> +	if (!rq) {
>  		lrbp->crypto_key_slot = -1;
>  		return;
>  	}
>  
> -	lrbp->crypto_key_slot = blk_crypto_keyslot_index(rq->crypt_keyslot);
> -	lrbp->data_unit_num = rq->crypt_ctx->bc_dun[0];
> +	if (rq->crypt_keyslot) {
> +		lrbp->crypto_key_slot = blk_crypto_keyslot_index(rq->crypt_keyslot);
> +		lrbp->data_unit_num = rq->crypt_ctx->bc_dun[0];
> +	} else if (rq->crypt_ctx && rq->crypt_ctx->bc_slot.data_unit_size_bits) {
> +		lrbp->crypto_key_slot = rq->crypt_ctx->bc_slot.phy_slot;
> +		lrbp->data_unit_num = rq->crypt_ctx->bc_dun[0];
> +	} else {
> +		lrbp->crypto_key_slot = -1;
> +	}
> +
>  }

[Severity: High]
Does this code silently bypass encryption if a malformed request is submitted?

If an IO request is submitted with a crypto context (rq->crypt_ctx != NULL) but
has data_unit_size_bits == 0, this logic falls through to the final else block
and sets crypto_key_slot to -1.

Since the block layer explicitly skips validation (bio_crypt_check_alignment())
for slot-based bios, could an untrusted guest VM submitting a virtio-blk
request with data_unit_size_bits == 0 cause an explicitly encrypted IO to be
written to the physical storage device as plaintext?

Should this path fail the request rather than falling open to plaintext,
perhaps by surfacing an error up to the caller so it can be failed with
BLK_STS_IOERR?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827160806.1295313-1-linlin.zhang@oss.qualcomm.com?part=6