Re: [PATCH v3 3/8] remoteproc: qcom: pas: register TMD thermal cooling devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Config QCOM_Q6V5_PAS and QCOM_QMI_TMD select QCOM_QMI_HELPERS without inheriting its dependency on NET, leading to Kconfig unmet dependency warnings and build failures.
- [High] In qmi_tmd_exit(), the QMI handle is released before unregistering the thermal cooling devices, exposing a race condition that leads to a Use-After-Free.
- [High] If thermal_of_cooling_device_register() fails, tmd->cdev holds an ERR_PTR which causes a kernel panic when passed to thermal_cooling_device_unregister().
--

commit 054962d4a1d3642bf94b2a883fba802967d5e58a
Author: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>

remoteproc: qcom: pas: register TMD thermal cooling devices

Add support for Thermal Mitigation Devices (TMDs) to enable thermal
throttling of remote processors through QMI. This enables the thermal
framework to request mitigation when remote subsystems contribute to thermal
issues.

> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 2caadbbcf8307..a292ce57fd4a4 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -128,6 +128,16 @@ config QCOM_QMI_HELPERS
>  	tristate
>  	depends on NET
>  
> +config QCOM_QMI_TMD
> +	bool "Qualcomm QMI TMD library" if COMPILE_TEST
> +	depends on ARCH_QCOM
> +	select QCOM_QMI_HELPERS

[Severity: High]
Does this code need to inherit the dependency on NET?

Looking at QCOM_QMI_HELPERS, it strictly depends on NET. If QCOM_QMI_TMD is
enabled while NET is disabled, Kconfig will forcefully enable
QCOM_QMI_HELPERS with unsatisfied dependencies. This could trigger unmet
direct dependencies warnings and link-time errors due to missing socket
APIs.

> diff --git a/drivers/soc/qcom/qmi_tmd.c b/drivers/soc/qcom/qmi_tmd.c
> new file mode 100644
> index 0000000000000..9d88ae48c8647
> --- /dev/null
> +++ b/drivers/soc/qcom/qmi_tmd.c
[ ... ]
> +static int qmi_tmd_register(struct qmi_tmd_client *qmi_tmd_cli,
> +			    const char *label, u8 max_state)
> +{
[ ... ]
> +	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);

[Severity: High]
Could this leave tmd->cdev containing an ERR_PTR if registration fails?

If thermal_of_cooling_device_register() returns an error, tmd->cdev will
hold that ERR_PTR. Later, when qmi_tmd_unregister() runs, the
if (!tmd->cdev) check won't catch the error pointer, causing
thermal_cooling_device_unregister() to dereference it and panic the kernel.

[ ... ]
> +void qmi_tmd_exit(struct qmi_tmd_client *qmi_tmd_cli)
> +{
> +	if (!qmi_tmd_cli)
> +		return;
> +
> +	cancel_work_sync(&qmi_tmd_cli->svc_arrive_work);
> +	qmi_handle_release(&qmi_tmd_cli->handle);
> +	qmi_tmd_unregister(qmi_tmd_cli);

[Severity: High]
Does releasing the QMI handle before unregistering the thermal devices
create a race condition?

If the thermal core invokes qmi_tmd_set_cur_state() asynchronously between
qmi_handle_release() and qmi_tmd_unregister(), qmi_tmd_send_state_request()
will try to use the already destroyed QMI handle, which could result in a
use-after-free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-qmi-tmd-v3-0-291a2ff4c634@oss.qualcomm.com?part=3