Re: [PATCH 4/8] media: venus: migrate firmware loading to qcom_mdt_pas_load() and qcom_pas_prepare_and_auth_reset()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: venus: devm_ allocation in venus_load_fw() causes leak on error recovery
--

commit 8d4223f26ac66cddad9e1923272c5c9297949631
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

media: venus: migrate firmware loading to qcom_mdt_pas_load() and qcom_pas_prepare_and_auth_reset()

This commit migrates the firmware loading for the TrustZone path to use the
new PAS context allocation and loading functions. It threads the PAS context
from venus_load_fw() through to venus_boot() where authentication occurs.

> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> index 3a38ff985822f..4ea4c7769ac3d 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c

[ ... ]

> @@ -117,21 +116,19 @@ static int venus_load_fw(struct venus_core *core, const char *fwname,
>  		goto err_release_fw;
>  	}
>  
> -	mem_va = memremap(*mem_phys, *mem_size, MEMREMAP_WC);
> -	if (!mem_va) {
> -		dev_err(dev, "unable to map memory region %pa size %#zx\n", mem_phys, *mem_size);
> -		ret = -ENOMEM;
> -		goto err_release_fw;
> -	}
> -
> -	if (core->use_tz)
> -		ret = qcom_mdt_load(dev, mdt, fwname, VENUS_PAS_ID,
> -				    mem_va, *mem_phys, *mem_size, NULL);
> -	else
> -		ret = qcom_mdt_load_no_init(dev, mdt, fwname, mem_va,
> +	if (core->use_tz) {
> +		core->fw.pas_ctx = devm_qcom_pas_context_alloc(dev, VENUS_PAS_ID,
> +							       *mem_phys, *mem_size);

[Severity: High]
Does this devm allocation leak memory during firmware error recovery?

Looking at the firmware recovery path, when a firmware crash occurs,
venus_sys_error_handler() repeatedly invokes venus_boot(), which then calls
venus_load_fw().

Because devm_qcom_pas_context_alloc() attaches the allocation to the device's
lifecycle, it is only freed when the device is unbound. Each recovery cycle
will allocate a new PAS context and overwrite core->fw.pas_ctx without freeing
the previous one. This could lead to unbounded memory consumption and an OOM
crash if an attacker or stream continuously triggers firmware crashes.

> +		if (IS_ERR(core->fw.pas_ctx)) {
> +			ret = PTR_ERR(core->fw.pas_ctx);
> +			core->fw.pas_ctx = NULL;
> +			goto err_release_fw;
> +		}
> +		ret = qcom_mdt_pas_load(core->fw.pas_ctx, mdt, fwname, NULL);
> +	} else {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925155737.4161358-1-mukesh.ojha@oss.qualcomm.com?part=4