Re: [PATCH v4 05/10] remoteproc: qcom_q6v5_pas: enable QMI TMD cooling support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Incorrect teardown order in `qmi_tmd_exit()` can lead to a Use-After-Free if `svc_arrive_work` is queued concurrently.
- [High] Failure in `thermal_of_cooling_device_register()` leaves an `ERR_PTR` in `tmd->cdev`, leading to a kernel panic during cleanup in `qmi_tmd_unregister()`.
--

commit 117e89cf6f52e4949e2dfdf865b41f6db55c4ceb
Author: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>

remoteproc: qcom_q6v5_pas: enable QMI TMD cooling support

Enable Thermal Mitigation Device (TMD) support for PAS-managed CDSP and
modem remote processors on platforms that expose the QMI TMD service.
This provides the mapping between DT cooling indices and QMI TMD names,
allowing remoteproc nodes to act as cooling devices in the thermal
framework.

> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index a4756a449e64b..e2cce4efec253 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c

[ ... ]

> @@ -1141,6 +1150,9 @@ static const struct qcom_pas_data sa8775p_cdsp0_resource = {
>  	.ssr_name = "cdsp",
>  	.sysmon_name = "cdsp",
>  	.ssctl_id = 0x17,
> +	.tmd_instance_id = 0x43,
> +	.tmd_name = cdsp_tmd_name,
> +	.num_tmd = ARRAY_SIZE(cdsp_tmd_name),

[Severity: High]
Could enabling TMD here expose a kernel panic during cleanup?

In drivers/soc/qcom/qmi_tmd.c:qmi_tmd_register(), if
thermal_of_cooling_device_register() fails, it returns an ERR_PTR which is
stored in tmd->cdev without being cleared:

qmi_tmd_register()
	tmd->cdev = thermal_of_cooling_device_register(dev->of_node, index,
						       label, tmd, &qmi_tmd_cooling_ops);
	if (IS_ERR(tmd->cdev))
		return PTR_ERR(tmd->cdev);

Later during error rollback or driver unbind, qmi_tmd_unregister() bypasses the
NULL check and unconditionally dereferences the error pointer:

qmi_tmd_unregister()
	if (!tmd->cdev)
		continue;

	thermal_cooling_device_unregister(tmd->cdev);

Can passing this ERR_PTR to thermal_cooling_device_unregister() lead to a
panic?

[Severity: Critical]
Is there a use-after-free regression introduced when enabling this feature?

In drivers/soc/qcom/qmi_tmd.c:qmi_tmd_exit(), the teardown order cancels
the work before releasing the QMI handle:

qmi_tmd_exit()
	cancel_work_sync(&qmi_tmd_cli->svc_arrive_work);
	qmi_handle_release(&qmi_tmd_cli->handle);

Since the QMI RX worker thread is not stopped until qmi_handle_release()
executes, a NEW_SERVER message arriving in this window can trigger
qmi_tmd_new_server() and re-queue the work:

qmi_tmd_new_server()
	queue_work(system_highpri_wq, &qmi_tmd_cli->svc_arrive_work);

Once qmi_tmd_exit() returns, the qmi_tmd_cli structure is freed, but the
pending work may execute later. Can this concurrent queueing cause a
use-after-free on qmi_tmd_cli?

>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-qmi-tmd-v4-0-3882189c1f83@oss.qualcomm.com?part=5