Re: [PATCH v3 4/5] dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing NULL check for `device_get_match_data()` return value in probe.
- [High] The driver registers a thermal sensor found at an arbitrary device tree index but completely ignores this index during hardware configuration, hardcoding register operations to sensor 0.
--

--- Patch [4]: [PATCH v3 4/5] dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300 ---
commit 52153439c55d2e646aae8a83ae212f55c09dcf24
Author: Binbin Zhou <zhoubinbin@loongson.cn>

dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300

This commit adds the loongson,ls2k0300-thermal compatible string to the
device tree bindings. It mandates the loongson,chipid property for this new
SoC variant to provide a syscon phandle for temperature calculation.

> diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> index 79e691b08341c..d312e27737a20 100644
> --- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> @@ -10,13 +10,11 @@ maintainers:
>    - zhanghongchen <zhanghongchen@loongson.cn>
>    - Yinbo Zhu <zhuyinbo@loongson.cn>
>  
> -allOf:
> -  - $ref: /schemas/thermal/thermal-sensor.yaml#
> -
>  properties:
>    compatible:
>      oneOf:
>        - enum:
> +          - loongson,ls2k0300-thermal
>            - loongson,ls2k1000-thermal
>            - loongson,ls2k2000-thermal
>        - items:

[ ... ]

[Severity: High]
This is a pre-existing issue, but can this code result in a null pointer
dereference?

If the driver is bound to a device via the sysfs bind interface without a
matching device tree node, device_get_match_data() returns null:

drivers/thermal/loongson2_thermal.c:loongson2_thermal_probe() {
    ...
	data->chip_data = device_get_match_data(dev);

	data->ctrl_reg = devm_platform_ioremap_resource(pdev, 0);
	if (IS_ERR(data->ctrl_reg))
		return PTR_ERR(data->ctrl_reg);

	/* The temperature output register is separate for Loongson-2K2000 */
	if (data->chip_data->flags & LS2K2000_THSENS_OUT_FLAG) {
    ...
}

Does this path blindly dereference data->chip_data via
data->chip_data->flags before any validation?

Could this allow a root user or privileged process writing to sysfs to crash
the kernel?

[Severity: High]
This is a pre-existing issue, but does this driver ignore the dynamically
discovered sensor index during hardware configuration?

The probe loop successfully registers the first available sensor index found
in the device tree, but doesn't store it to update the internal sensor
selection state:

drivers/thermal/loongson2_thermal.c:loongson2_thermal_probe() {
    ...
	for (i = 0; i <= LOONGSON2_MAX_SENSOR_SEL_NUM; i++) {
		tzd = devm_thermal_of_zone_register(dev, i, data, data->chip_data->thermal_ops);
		if (!IS_ERR(tzd))
			break;
    ...
}

Later, the hardware operations calculate the offset using the statically
hardcoded thermal_sensor_sel from the constant match data:

drivers/thermal/loongson2_thermal.c:loongson2_set_ctrl_regs() {
    ...
	int reg_off  = data->chip_data->thermal_sensor_sel * 2;
    ...
}

Will this always configure temperature thresholds and read temperatures for
sensor 0, even if a thermal zone was registered for a different sensor index?

A related patch in this series (thermal/drivers/loongson2: Correct thermal
sensor registration loop) actively enables this buggy path to be reached by
continuing on -ENODEV instead of aborting.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785480242.git.zhoubinbin@loongson.cn?part=4