Re: [PATCH 6/8] wifi: ath12k: 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 2 potential issue(s) to consider:
- [High] leak: devm_qcom_pas_context_alloc memory leak in recovery loop
- [High] leak: TrustZone memory leak via missing qcom_pas_metadata_release
--

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

wifi: ath12k: migrate firmware loading to qcom_mdt_pas_load() and qcom_pas_prepare_and_auth_reset()

ath12k_ahb_power_up() and ath12k_ahb_load_auth_shared_fw() call
qcom_mdt_load() for SCM-authenticated paths and then
qcom_pas_auth_and_reset() which skips the shmbridge prepare step
required before TrustZone authentication.

> diff --git a/drivers/net/wireless/ath/ath12k/ahb.c b/drivers/net/wireless/ath/ath12k/ahb.c
> index 85360365aa4b2..6f0ad29a83f85 100644
> --- a/drivers/net/wireless/ath/ath12k/ahb.c
> +++ b/drivers/net/wireless/ath/ath12k/ahb.c

[ ... ]

> @@ -424,14 +425,20 @@ static int ath12k_ahb_power_up(struct ath12k_base *ab)
>  		ATH12K_AHB_UPD_SWID;
>  
>  	/* Load FW image to a reserved memory location */
> -	if (ab_ahb->scm_auth_enabled)
> -		ret = qcom_mdt_load(dev, fw, fw_name, pasid, rproc_info->mem_region,
> -				    rproc_info->mem_phys, rproc_info->mem_size,
> -				    NULL);
> -	else
> +	if (ab_ahb->scm_auth_enabled) {
> +		ctx = devm_qcom_pas_context_alloc(dev, pasid,
> +						  rproc_info->mem_phys,
> +						  rproc_info->mem_size);

[Severity: High]
Is it safe to use a devm-managed allocation here?

Since ath12k_ahb_power_up() is called dynamically during normal device
operation, such as in firmware crash recovery loops, and the platform device
does not unbind during this process, does this permanently leak a
struct qcom_pas_context on every power-up cycle?

> +		if (IS_ERR(ctx)) {
> +			ret = PTR_ERR(ctx);
> +			goto err_fw;
> +		}
> +		ret = qcom_mdt_pas_load(ctx, fw, fw_name, NULL);

[Severity: High]
Does this code leak TrustZone memory?

qcom_mdt_pas_load() allocates firmware metadata inside TrustZone. The
caller is typically required to invoke qcom_pas_metadata_release() to
free this secure memory after authentication is complete or on error.

Without a call to qcom_pas_metadata_release() here or later in the
function, is this secure memory permanently lost on every firmware load?

> +	} else {
>  		ret = qcom_mdt_load_no_init(dev, fw, fw_name, rproc_info->mem_region,
>  					    rproc_info->mem_phys, rproc_info->mem_size,
>  					    NULL);
> +	}

[ ... ]

> @@ -1012,14 +1020,19 @@ static int ath12k_ahb_load_auth_shared_fw(struct ath12k_base *ab,
>  	ath12k_dbg(ab, ATH12K_DBG_AHB, "loading firmware %s, size %zd\n", fw_name,
>  		   fw->size);
>  
> -	ret = qcom_mdt_load(ab->dev, fw, fw_name, pasid, rproc_info->mem_region,
> -			    rproc_info->mem_phys, rproc_info->mem_size, NULL);
> +	ctx = devm_qcom_pas_context_alloc(ab->dev, pasid,
> +					  rproc_info->mem_phys,
> +					  rproc_info->mem_size);
> +	if (IS_ERR(ctx))
> +		return PTR_ERR(ctx);
> +
> +	ret = qcom_mdt_pas_load(ctx, fw, fw_name, NULL);

[Severity: High]
Similar to the question above, is a call to qcom_pas_metadata_release()
needed here to prevent leaking TrustZone memory after the shared
firmware is loaded and authenticated?

>  	if (ret) {
>  		ath12k_err(ab, "failed to load RO firmware %s: %d\n", fw_name, ret);
>  		return ret;
>  	}

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