Re: [PATCH v4 07/10] arm64: dts: qcom: lemans: Enable CDSP cooling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing NULL reset of tmd->cdev on registration failure causes invalid pointer dereference during unregistration and breaks subsequent registrations.
- [High] Destroying the QMI handle before unregistering the thermal cooling devices introduces a race condition with userspace resulting in a Use-After-Free.
--

commit 68055a14c90dc23e8743fc49a13b0805f601355b
Author: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
Subject: arm64: dts: qcom: lemans: Enable CDSP cooling

This commit sets up CDSP cooling for both instances in the lemans device tree.
It configures the cooling maps and cooling devices to throttle the CDSP when it
reaches 105°C.

> diff --git a/arch/arm64/boot/dts/qcom/lemans.dtsi b/arch/arm64/boot/dts/qcom/lemans.dtsi
> index 353a6e6fd3acb..89fa25e9d3c6d 100644
> --- a/arch/arm64/boot/dts/qcom/lemans.dtsi
> +++ b/arch/arm64/boot/dts/qcom/lemans.dtsi
[ ... ]
> @@ -8764,7 +8769,7 @@ nsp-0-0-0-thermal {
>  			thermal-sensors = <&tsens2 5>;
>  
>  			trips {
> -				trip-point0 {
> +				nsp_0_0_0_alert0: trip-point0 {
>  					temperature = <105000>;
>  					hysteresis = <5000>;
>  					type = "passive";
> @@ -8776,6 +8781,14 @@ trip-point1 {
>  					type = "passive";
>  				};
>  			};
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&nsp_0_0_0_alert0>;
> +					cooling-device = <&remoteproc_cdsp0 QCOM_CDSP_TMD_CDSP_SW
> +							 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +			};
>  		};

[Severity: High]
Does enabling this cooling device expose a regression involving an invalid
pointer dereference in qmi_tmd_register()?

If thermal_of_cooling_device_register() fails, tmd->cdev holds an ERR_PTR:

drivers/soc/qcom/qmi_tmd.c: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);
    ...
}

Because tmd->cdev is not reset to NULL, qmi_tmd_unregister() will pass this
ERR_PTR to thermal_cooling_device_unregister(), which expects a valid
pointer or NULL:

drivers/soc/qcom/qmi_tmd.c:qmi_tmd_unregister() {
    ...
		if (!tmd->cdev)
			continue;

		thermal_cooling_device_unregister(tmd->cdev);
    ...
}

Will this cause a kernel panic when the thermal core dereferences the
invalid pointer?

Additionally, if the QMI service restarts, would a stale error pointer
silently skip registration entirely?

[ ... ]
> @@ -8784,7 +8797,7 @@ nsp-0-1-0-thermal {
>  			thermal-sensors = <&tsens2 6>;
>  
>  			trips {
> -				trip-point0 {
> +				nsp_0_1_0_alert0: trip-point0 {
>  					temperature = <105000>;
>  					hysteresis = <5000>;
>  					type = "passive";
> @@ -8796,6 +8809,14 @@ trip-point1 {
>  					type = "passive";
>  				};
>  			};
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&nsp_0_1_0_alert0>;
> +					cooling-device = <&remoteproc_cdsp0 QCOM_CDSP_TMD_CDSP_SW
> +							 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +			};
>  		};

[Severity: High]
Could exposing these cooling devices to userspace trigger a use-after-free
regression during teardown?

In qmi_tmd_exit(), the QMI handle is released before the cooling devices
are unregistered:

drivers/soc/qcom/qmi_tmd.c:qmi_tmd_exit() {
    ...
	qmi_handle_release(&qmi_tmd_cli->handle);
	qmi_tmd_unregister(qmi_tmd_cli);

	scoped_guard(mutex, &qmi_tmd_cli->mutex)
		qmi_tmd_cli->connection_active = false;
    ...
}

If a concurrent write to the thermal sysfs node invokes
qmi_tmd_set_cur_state() -> qmi_tmd_send_state_request() before the cooling
device is unregistered, the connection_active flag is still true.

Would this cause qmi_txn_init() and qmi_send_request() to operate on the
already destroyed qmi_tmd_cli->handle?

Should qmi_tmd_unregister() be called before releasing the QMI handle to
ensure pending sysfs operations are safely quiesced?

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