[PATCH] thermal/drivers/sprd: validate the sensor id from the device tree

[PATCH] thermal/drivers/sprd: validate the sensor id from the device tree

[Weigang He]

sprd_thm_probe() reads each sensor child node's "reg" property into
sen->id and uses it without validation: to compute the sensor's
register offsets in sprd_thm_sensor_init(), as the thermal zone id, and
finally as an index into thm->sensor[], which has SPRD_THM_MAX_SENSOR
(8) entries:

	ret = of_property_read_u32(sen_child, "reg", &sen->id);
	...
	thm->sensor[sen->id] = sen;

Only the number of sensor nodes is checked against SPRD_THM_MAX_SENSOR.
A "reg" value of 8 or more, or one that is negative once stored in the
int sen->id, makes the register offsets point outside the sensor range
and the final store land outside thm->sensor[], overwriting the fields
that follow it in struct sprd_thermal_data or other memory.

Reject an out-of-range id right after reading it, before it is used.

This does not address sparse ids, such as a single sensor with id 7:
they leave holes in thm->sensor[] that the later loops over
0..nr_sensors-1 dereference. That needs a separate fix.

Found by static analysis tool CodeQL.

Fixes: 554fdbaf19b1 ("thermal: sprd: Add Spreadtrum thermal driver support")
Assisted-by: LLM codeql
Signed-off-by: Weigang He <geoffreyhe2@gmail.com>
---

Notes:
    Compile-tested only (ARCH=arm64 allmodconfig, W=1). Not tested on
    hardware, and there is no reproducer.
    
    The CodeQL query behind this report was synthesized with LLM assistance,
    and the fix and changelog were drafted with LLM assistance; I have
    reviewed them.

 drivers/thermal/sprd_thermal.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/thermal/sprd_thermal.c b/drivers/thermal/sprd_thermal.c
index d683fcb0f8ab8..b57277e2eac01 100644
--- a/drivers/thermal/sprd_thermal.c
+++ b/drivers/thermal/sprd_thermal.c
@@ -396,6 +396,12 @@ static int sprd_thm_probe(struct platform_device *pdev)
 			goto of_put;
 		}
 
+		if (sen->id < 0 || sen->id >= SPRD_THM_MAX_SENSOR) {
+			dev_err(&pdev->dev, "invalid sensor id %d\n", sen->id);
+			ret = -EINVAL;
+			goto of_put;
+		}
+
 		ret = sprd_thm_sensor_calibration(sen_child, thm, sen);
 		if (ret) {
 			dev_err(&pdev->dev, "efuse cal analysis failed");

base-commit: 165768bb70265b5c38cf0b73fafd75be235f8b14
prerequisite-patch-id: c5a3be8688fd8e88a00352acb1374e91fcb52a03
prerequisite-patch-id: 67693e2c08624df0841619cc085ce9200f4385fc
prerequisite-patch-id: f3d73f7c19be7e952aa8061303f53f9a08576a88
prerequisite-patch-id: 541e578709d048f4c8115f1d926be2f2c03ecb0e
prerequisite-patch-id: f23f8e0693435497645805822d98a92e57fb46f3
prerequisite-patch-id: eae82895db8ba67018a777a91a283ad6bc4a55b2
-- 
2.43.0

Re: [PATCH] thermal/drivers/sprd: validate the sensor id from the device tree

[Daniel Lezcano]

On 9/26/26 06:27, Weigang He wrote:
> sprd_thm_probe() reads each sensor child node's "reg" property into
> sen->id and uses it without validation: to compute the sensor's
> register offsets in sprd_thm_sensor_init(), as the thermal zone id, and
> finally as an index into thm->sensor[], which has SPRD_THM_MAX_SENSOR
> (8) entries:
> 
> 	ret = of_property_read_u32(sen_child, "reg", &sen->id);
> 	...
> 	thm->sensor[sen->id] = sen;
> 
> Only the number of sensor nodes is checked against SPRD_THM_MAX_SENSOR.
> A "reg" value of 8 or more, or one that is negative once stored in the
> int sen->id, makes the register offsets point outside the sensor range
> and the final store land outside thm->sensor[], overwriting the fields
> that follow it in struct sprd_thermal_data or other memory.
> 
> Reject an out-of-range id right after reading it, before it is used.
> 
> This does not address sparse ids, such as a single sensor with id 7:
> they leave holes in thm->sensor[] that the later loops over
> 0..nr_sensors-1 dereference. That needs a separate fix.
> 
> Found by static analysis tool CodeQL.
> 
> Fixes: 554fdbaf19b1 ("thermal: sprd: Add Spreadtrum thermal driver support")
> Assisted-by: LLM codeql
> Signed-off-by: Weigang He <geoffreyhe2@gmail.com>
> ---
> 
> Notes:
>      Compile-tested only (ARCH=arm64 allmodconfig, W=1). Not tested on
>      hardware, and there is no reproducer.
>      
>      The CodeQL query behind this report was synthesized with LLM assistance,
>      and the fix and changelog were drafted with LLM assistance; I have
>      reviewed them.
> 
>   drivers/thermal/sprd_thermal.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/thermal/sprd_thermal.c b/drivers/thermal/sprd_thermal.c
> index d683fcb0f8ab8..b57277e2eac01 100644
> --- a/drivers/thermal/sprd_thermal.c
> +++ b/drivers/thermal/sprd_thermal.c
> @@ -396,6 +396,12 @@ static int sprd_thm_probe(struct platform_device *pdev)
>   			goto of_put;
>   		}
>   
> +		if (sen->id < 0 || sen->id >= SPRD_THM_MAX_SENSOR) {
> +			dev_err(&pdev->dev, "invalid sensor id %d\n", sen->id);
> +			ret = -EINVAL;
> +			goto of_put;
> +		}
> +

Why is there a mismatch between this check and above in the code:

         thm->nr_sensors = of_get_child_count(np);
         if (thm->nr_sensors == 0 || thm->nr_sensors > 
SPRD_THM_MAX_SENSOR) {
                 dev_err(&pdev->dev, "incorrect sensor count\n");
                 return -EINVAL;
         }

(note the "thm->nr_sensors > SPRD_THM_MAX_SENSOR" instead of 
"thm->nr_sensors >*=* SPRD_THM_MAX_SENSOR"


>   		ret = sprd_thm_sensor_calibration(sen_child, thm, sen);
>   		if (ret) {
>   			dev_err(&pdev->dev, "efuse cal analysis failed");
> 
> base-commit: 165768bb70265b5c38cf0b73fafd75be235f8b14
> prerequisite-patch-id: c5a3be8688fd8e88a00352acb1374e91fcb52a03
> prerequisite-patch-id: 67693e2c08624df0841619cc085ce9200f4385fc
> prerequisite-patch-id: f3d73f7c19be7e952aa8061303f53f9a08576a88
> prerequisite-patch-id: 541e578709d048f4c8115f1d926be2f2c03ecb0e
> prerequisite-patch-id: f23f8e0693435497645805822d98a92e57fb46f3
> prerequisite-patch-id: eae82895db8ba67018a777a91a283ad6bc4a55b2

Re: [PATCH] thermal/drivers/sprd: validate the sensor id from the device tree

[he geoffrey]

On 9/29/26 10:46, Daniel Lezcano wrote:
> Why is there a mismatch between this check and above in the code:
>
>          thm->nr_sensors = of_get_child_count(np);
>          if (thm->nr_sensors == 0 || thm->nr_sensors > SPRD_THM_MAX_SENSOR) {
>
> (note the "thm->nr_sensors > SPRD_THM_MAX_SENSOR" instead of
> "thm->nr_sensors >= SPRD_THM_MAX_SENSOR"

Hi Daniel,

Thanks for the review.

The two checks bound different things. nr_sensors is a count, so eight
sensors is still valid and only a count above SPRD_THM_MAX_SENSOR is
rejected. sen->id is an index into thm->sensor[SPRD_THM_MAX_SENSOR], so
it must be below SPRD_THM_MAX_SENSOR, hence ">=".

Thanks,
Weigang