Re: [PATCH v6 11/16] media: qcom: Switch to generic PAS TZ APIs

[Sumit Garg <sumit.garg@kernel.org>]

On Tue, May 19, 2026 at 07:34:04AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Unconditional use of SCM-specific API (qcom_scm_mem_protect_video_var) will cause a NULL pointer dereference when using a non-SCM TrustZone backend.
> - [High] Migrating to the generic PAS API exposes a TOCTOU race and Use-After-Free due to a lack of backend provider lifetime management.
> --
> 
> commit 2d2fe5038803b9390438e02f3c0f53b945cc9bdc
> Author: Sumit Garg <sumit.garg@oss.qualcomm.com>
> 
> media: qcom: Switch to generic PAS TZ APIs
> 
> This commit switches the Qualcomm media client drivers to use the generic
> PAS TZ APIs instead of the SCM-specific ones. This allows the drivers to
> support multiple TrustZone backend implementations such as QTEE and OP-TEE.
> 
> > diff --git a/drivers/media/platform/qcom/iris/iris_firmware.c b/drivers/media/platform/qcom/iris/iris_firmware.c
> > index 5f408024e967f..b3c5281aea919 100644
> > --- a/drivers/media/platform/qcom/iris/iris_firmware.c
> > +++ b/drivers/media/platform/qcom/iris/iris_firmware.c
> [ ... ]
> > @@ -79,7 +80,7 @@ int iris_fw_load(struct iris_core *core)
> >  		return -ENOMEM;
> >  	}
> >  
> > -	ret = qcom_scm_pas_auth_and_reset(core->iris_platform_data->pas_id);
> > +	ret = qcom_pas_auth_and_reset(core->iris_platform_data->pas_id);
> 
> Does moving to the generic PAS API introduce a race condition with module
> unloading?
> 
> Looking at the generic PAS wrapper qcom_pas_auth_and_reset() in
> drivers/firmware/qcom/qcom_pas.c:
> 
> int qcom_pas_auth_and_reset(u32 pas_id)
> {
> 	if (!ops_ptr)
> 		return -ENODEV;
> 
> 	return ops_ptr->auth_and_reset(ops_ptr->dev, pas_id);
> }
> 
> If the PAS backend module is unloaded concurrently, ops_ptr could be set to
> null after the check but before the function pointer is invoked. Do the
> client drivers need to hold a reference count to pin the backend provider
> in memory before calling these APIs to avoid a use-after-free?

The PAS backend is not expected to be unloaded for the entire boot power
cycle. This is the same behavior as the existing SCM backend, so not an
issue.

> 
> [ ... ]
> > diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> > index 1de7436713ed9..3a38ff985822f 100644
> > --- a/drivers/media/platform/qcom/venus/firmware.c
> > +++ b/drivers/media/platform/qcom/venus/firmware.c
> [ ... ]
> > @@ -218,7 +219,7 @@ int venus_boot(struct venus_core *core)
> >  	int ret;
> >  
> >  	if (!IS_ENABLED(CONFIG_QCOM_MDT_LOADER) ||
> > -	    (core->use_tz && !qcom_scm_is_available()))
> > +	    (core->use_tz && !qcom_pas_is_available()))
> >  		return -EPROBE_DEFER;
> 
> If the platform uses a non-SCM TrustZone backend like OP-TEE,
> qcom_pas_is_available() will return true, but the qcom_scm subsystem
> might not be probed.

qcom_scm is also probed based on DT alongside OP-TEE to support the
other SCM calls which are compliant to SiP SMC specification. So not an
issue.

> 
> Will this lead to a null pointer dereference later in venus_boot() when
> it calls the SCM-specific API?
> 
> venus_boot() {
>     ...
> 	if (core->use_tz && res->cp_size) {
> 		ret = qcom_scm_mem_protect_video_var(res->cp_start,
> 						     res->cp_size,
> 						     res->cp_nonpixel_start,
> 						     res->cp_nonpixel_size);
> 	}
>     ...
> }
> 
> Looking at qcom_scm_mem_protect_video_var(), it executes
> qcom_scm_call(__scm->dev, ...). If __scm is null because the SCM
> subsystem is absent, this dereference might cause a panic.

SCM subsystem is always present, it's only the PAS calls being handled
by this OP-TEE backend now. So, no case of a panic.

-Sumit

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