Re: [PATCH v8 09/14] media: qcom: 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] Calling `qcom_scm_mem_protect_video_var()` without ensuring SCM is available causes a NULL pointer dereference.
- [High] Transition to generic PAS API introduces a Use-After-Free race condition due to missing module reference management.
--

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

media: qcom: Switch to generic PAS TZ APIs

Switch qcom media client drivers over to generic PAS TZ APIs. Generic PAS
TZ service allows to support multiple TZ implementation backends like QTEE
based SCM PAS service, OP-TEE based PAS service and any further future TZ
backend service.

> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> --- 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;

[Severity: High]
By replacing qcom_scm_is_available() with qcom_pas_is_available(), we no
longer guarantee that the SCM driver has probed.

However, later in this function (and similarly in iris_fw_load), we still
unconditionally call the SCM-specific API:

drivers/media/platform/qcom/venus/firmware.c:venus_boot() {
    ...
    ret = qcom_scm_mem_protect_video_var(res->cp_start,
                                         res->cp_size,
                                         res->cp_nonpixel_start,
                                         res->cp_nonpixel_size);
    ...
}

If a non-SCM backend like OP-TEE is used, will this cause a NULL pointer
dereference in qcom_scm_call() because the global __scm pointer was never
initialized?

drivers/firmware/qcom/qcom_scm.c:qcom_scm_mem_protect_video_var() {
    ...
    ret = qcom_scm_call(__scm->dev, &desc, &res);
    ...
}

[ ... ]

> @@ -236,7 +237,7 @@ int venus_boot(struct venus_core *core)
>  	core->fw.mem_phys = mem_phys;
>  
>  	if (core->use_tz)
> -		ret = qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
> +		ret = qcom_pas_auth_and_reset(VENUS_PAS_ID);

[Severity: High]
Does switching to the generic PAS API introduce a use-after-free race
condition if the backend module is unloaded concurrently?

Looking at the generic PAS implementation:

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 the generic PAS backend is built as a module and unloaded by userspace
right after the ops_ptr check passes, could ops_ptr->auth_and_reset jump to
unmapped memory?

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