Re: [PATCH v2 2/2] thermal/drivers/qcom/tsens: Add support for ipq9650 tsens

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing out-of-bounds array read in tsens_probe() when DT overrides `#qcom,sensors`.
--

commit 9a11fba8da339febaef9c47f6f85fc5817527502
Author: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>

thermal/drivers/qcom/tsens: Add support for ipq9650 tsens

This commit adds thermal sensor (tsens) support for the ipq9650 platform by
reusing the ipq5332 data and modifying the sensor-related information to
accommodate 11 sensors.

> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
> index 8d9698ea3ec40..ef1fee2266a32 100644
> --- a/drivers/thermal/qcom/tsens-v2.c
> +++ b/drivers/thermal/qcom/tsens-v2.c
> @@ -300,6 +300,14 @@ const struct tsens_plat_data data_ipq5424 = {
>  	.fields		= tsens_v2_regfields,
>  };
>  
> +const struct tsens_plat_data data_ipq9650 = {
> +	.num_sensors	= 11,
> +	.ops		= &ops_ipq5332,
> +	.hw_ids		= (unsigned int []){5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15},

[Severity: High]
This is a pre-existing issue, but I wanted to ask about how hw_ids[] is
handled if the device tree specifies more sensors than data->num_sensors.

In tsens_probe() in drivers/thermal/qcom/tsens.c, the num_sensors value is
initially set to data->num_sensors, but can be overridden by the device
tree:

drivers/thermal/qcom/tsens.c:tsens_probe() {
    ...
    num_sensors = data->num_sensors;

    if (np)
        of_property_read_u32(np, "#qcom,sensors", &num_sensors);
    ...
}

If the overridden num_sensors is larger than data->num_sensors, the
initialization loop will iterate up to the new priv->num_sensors and read
from the statically-sized data->hw_ids[] array:

drivers/thermal/qcom/tsens.c:tsens_probe() {
    ...
    for (i = 0;  i < priv->num_sensors; i++) {
        if (data->hw_ids)
            priv->sensor[i].hw_id = data->hw_ids[i];
    ...
}

Could this result in an out-of-bounds read if the device tree provides a
malformed or unusually large value for #qcom,sensors, since the hw_ids[]
array here only has 11 elements?

Reading past the end could assign garbage values to hw_id, which later
might cause undefined behavior in functions like get_temp_tsens_valid()
that use hw_id in bit shifts.

> +	.feat		= &ipq5332_feat,
> +	.fields		= tsens_v2_regfields,
> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610081241.1468507-1-varadarajan.narayanan@oss.qualcomm.com?part=2