Re: [PATCH v4 04/10] remoteproc: qcom: pas: add support for TMD thermal cooling devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] A race condition between `qmi_tmd_exit` and `qmi_tmd_new_server` can lead to a use-after-free of `qmi_tmd_cli`.
- [High] TMD sysfs cooling device callbacks can race with teardown, leading to use-after-free on the QMI transaction IDR.
- [High] Data race on `tmd->cur_state` during QMI server reconnect can result in the remote processor receiving an outdated cooling state.
- [Medium] QCOM_Q6V5_PAS unconditionally selects QCOM_QMI_TMD, which lacks a COMPILE_TEST dependency and indirectly depends on NET, leading to Kconfig unmet dependency warnings and potential build failures.
--

commit 770900471517951fb204f7c5cc18bd57f37681a8
Author: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>

remoteproc: qcom: pas: add support for TMD thermal cooling devices

This commit registers Thermal Mitigation Devices (TMDs) for PAS-managed
remote processors to enable thermal throttling through QMI. It allows
the thermal framework to request mitigation when remote subsystems
contribute to thermal pressure.

> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index c521c744e7dbe..4fb2163c06556 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -228,6 +228,7 @@ config QCOM_Q6V5_PAS
>  	select QCOM_PIL_INFO
>  	select QCOM_MDT_LOADER
>  	select QCOM_Q6V5_COMMON
> +	select QCOM_QMI_TMD

[Severity: Medium]
Does this unconditionally select QCOM_QMI_TMD without fulfilling its
underlying ARCH_QCOM and NET dependencies?

According to Kconfig subsystem guidelines, selecting a config symbol without
inheriting its dependencies can cause Kconfig unmet dependency warnings and
potential build failures on architectures missing networking infrastructure
or ARCH_QCOM.

>  	select QCOM_RPROC_COMMON
>  	select QCOM_SCM
>  	help
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 808e9609988d3..a4756a449e64b 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
[ ... ]
> @@ -730,6 +749,66 @@ static void qcom_pas_unassign_memory_region(struct qcom_pas *pas)
[ ... ]
> +static int qcom_pas_setup_tmd(struct qcom_pas *pas, const struct qcom_pas_data *desc)
> +{
[ ... ]
> +	tmd_inst = qmi_tmd_init(pas->dev, desc->tmd_instance_id, tmd_names, desc->num_tmd);

[Severity: High]
Can a data race on tmd->cur_state during a QMI server reconnect result in the
remote processor receiving an outdated cooling state?

Looking at qmi_tmd_register() in drivers/soc/qcom/qmi_tmd.c:

	if (tmd->cdev)
		return qmi_tmd_send_state_request(tmd, tmd->cur_state);

The current state is read locklessly without holding cdev->lock. This
introduces a data race with concurrent sysfs updates to the cooling device's
state, which could result in sending an outdated mitigation state over QMI.

> +	if (IS_ERR(tmd_inst)) {
> +		ret = PTR_ERR(tmd_inst);
[ ... ]
> @@ -852,12 +931,21 @@ static int qcom_pas_probe(struct platform_device *pdev)
[ ... ]
> +remove_setup_tmd:
> +	if (pas->tmd_inst)
> +		qmi_tmd_exit(pas->tmd_inst);

[Severity: High]
Could a race condition between qmi_tmd_exit() and qmi_tmd_new_server() lead
to a use-after-free of qmi_tmd_cli?

Looking at qmi_tmd_exit() in drivers/soc/qcom/qmi_tmd.c:

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

The work queue is canceled before shutting down incoming QMI notifications via
qmi_handle_release(). If qmi_tmd_new_server() is invoked concurrently, it
could re-queue svc_arrive_work on the global system_highpri_wq, which then
executes after qmi_tmd_cli has been freed.

[Severity: High]
Can TMD sysfs cooling device callbacks race with teardown, leading to a
use-after-free on the QMI transaction IDR?

In qmi_tmd_exit():

	qmi_handle_release(&qmi_tmd_cli->handle);
	qmi_tmd_unregister(qmi_tmd_cli);

The QMI handle (and its IDR) is released before qmi_tmd_unregister() removes
the thermal cooling devices from sysfs. Meanwhile, connection_active is still
left as true. A concurrent sysfs write to the cur_state file exactly when the
driver is being removed would attempt to call idr_alloc_cyclic() on the
destroyed IDR.

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