Re: [PATCH v1 02/11] soc: qcom: add crypto_virt backend for virtio-blk inline crypto

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing `key_type` validation and endianness conversion in `crypto_virt_program_key()` allows raw keys to be programmed incorrectly.
- [High] `keyslot_program` callback triggers a `GFP_KERNEL` memory allocation on the bio submission path, causing a potential memory reclaim deadlock.
- [Medium] Kernel log spam (DoS) via unratelimited error printing on invalid key derivation.
--

commit 2e7401200408cdbaa703d51ece5e8968e39837e3
Author: linlzhan <linlin.zhang@oss.qualcomm.com>

soc: qcom: add crypto_virt backend for virtio-blk inline crypto

The commit adds the QCOM_CRYPTO_VIRT driver to support virtio-blk inline
crypto in a Qualcomm GVM environment. It maps keyslot programming and eviction
to SCM calls, offloading hardware operations to Trust Zone.

> diff --git a/drivers/soc/qcom/crypto_virt.c b/drivers/soc/qcom/crypto_virt.c
> new file mode 100644
> index 0000000000000..4ee2a36af6c1f
> --- /dev/null
> +++ b/drivers/soc/qcom/crypto_virt.c
> @@ -0,0 +1,89 @@

[ ... ]

> +static int crypto_virt_program_key(const struct blk_crypto_key *key,
> +				   unsigned int slot)
> +{
> +	u32 dus_512_units;
> +	int ret;

[ ... ]

> +	/* qcom_scm_ice_set_key()'s data_unit_size is expressed in 512-byte units */
> +	dus_512_units = key->crypto_cfg.data_unit_size / 512;
> +
> +	ret = qcom_scm_ice_set_key(slot, key->bytes, key->size,
> +		QCOM_SCM_ICE_CIPHER_AES_256_XTS, dus_512_units);

[Severity: High]
Does this code need to validate that the key is a hardware-wrapped key, or apply
the hardware-mandated endianness swap for standard keys before passing the key
buffer directly to the SCM call?

[Severity: High]
Could this SCM call, which allocates memory internally via
qcom_tzmem_alloc(..., GFP_KERNEL), trigger a memory reclaim deadlock?

Since this function serves as the keyslot_program profile callback, it executes
in the IO submission path where FS/IO reclaim is strictly forbidden. Should
this allocation be wrapped with memalloc_noio_save() to prevent direct reclaim
from attempting another bio submission?

> +	if (ret)
> +		pr_err("%s: slot=%u ret=%d\n", __func__, slot, ret);
> +
> +	return ret;
> +}

[ ... ]

> +static int crypto_virt_derive_sw_secret_key(const u8 *eph_key, size_t eph_key_size,
> +					    u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
> +{
> +	int ret;
> +
> +	ret = qcom_scm_derive_sw_secret(eph_key, eph_key_size,
> +					sw_secret, BLK_CRYPTO_SW_SECRET_SIZE);
> +	if (ret == -EIO || ret == -EINVAL)
> +		ret = -EBADMSG; /* probably invalid key */
> +
> +	if (ret)
> +		pr_err("%s: ret=%d\n", __func__, ret);

[Severity: Medium]
Could this unratelimited error print in the direct execution path of a
user-triggerable key derivation failure allow an attacker to trivially spam
the kernel logs?

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