Re: [PATCH v6 10/16] drm/msm: Switch to generic PAS TZ APIs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Infinite probe deferral on devices that do not support PAS.
- [High] Time-of-Check to Time-of-Use (TOCTOU) race leading to NULL pointer dereference or Use-After-Free.
--

commit 168f850440ba0271d11e8bfd4d88f6adbd042e93
Author: Sumit Garg <sumit.garg@oss.qualcomm.com>

drm/msm: Switch to generic PAS TZ APIs

This commit switches the drm/msm client drivers to use the generic PAS TZ
APIs instead of the SCM-specific ones. This allows the driver to support
multiple TrustZone implementation backends.

> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 66f80f2d12f95..6d68edf0578c2 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -146,10 +147,10 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
>  		goto out;
>  
>  	/* Send the image to the secure world */
> -	ret = qcom_scm_pas_auth_and_reset(pasid);
> +	ret = qcom_pas_auth_and_reset(pasid);

Could this introduce a race condition if the PAS backend is unbound
concurrently?

Looking at the new generic PAS APIs, they do not appear to protect the backend
operations pointer:

drivers/firmware/qcom/qcom_pas.c:qcom_pas_auth_and_reset() {
	if (!ops_ptr)
		return -ENODEV;

	return ops_ptr->auth_and_reset(ops_ptr->dev, pas_id);
}

If a dynamic backend like qcom_pas_tee is unbound via sysfs between the check
and the dereference, ops_ptr could become NULL, leading to a NULL pointer
dereference. 

Is there a mechanism preventing this race when switching from the SCM backend
to the generic PAS API?

[ ... ]

> @@ -175,9 +176,9 @@ int adreno_zap_shader_load(struct msm_gpu *gpu, u32 pasid)
>  	if (!zap_available)
>  		return -ENODEV;
>  
> -	/* We need SCM to be able to load the firmware */
> -	if (!qcom_scm_is_available()) {
> -		DRM_DEV_ERROR(&pdev->dev, "SCM is not available\n");
> +	/* We need PAS to be able to load the firmware */
> +	if (!qcom_pas_is_available()) {
> +		DRM_DEV_ERROR(&pdev->dev, "Qcom PAS is not available\n");
>  		return -EPROBE_DEFER;
>  	}

Will this cause an infinite probe deferral on platforms that do not support
PAS hardware?

The SCM backend skips registering PAS ops if the hardware lacks PAS support.
If qcom_pas_is_available() is called on such a platform, it returns false
because no backend has registered.

Since this check happens before zap_shader_load_mdt() parses the device tree
to determine if a zap shader is actually needed (which is what updates the
zap_available flag), won't the GPU probe indefinitely defer instead of
failing gracefully?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518072856.22790-1-sumit.garg@kernel.org?part=10