Re: [PATCH v3 16/19] thermal: qcom: tsens: Add support for MSM8952 tsens

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] thermal: tsens: Use-after-free in tsens_probe() via __init section mismatch
- [High] thermal: tsens: Heap buffer overflow in init_8952() via DT override
--

commit 5408ae137c176a5df112132baa926440df23a1b3
Author: Muzaffer Kadir <muzafferkadir@mainlining.org>

thermal: qcom: tsens: Add support for MSM8952 tsens

This commit adds TSENS thermal sensor support for the Qualcomm MSM8952
platform. It introduces calibration data for 11 sensors and wires up the
associated platform data structure.

Link: https://android.googlesource.com/kernel/msm.git/+/refs/tags/android-wear-7.0.0_r0.3/drivers/thermal/msm-tsens.c#1621

> diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
> index faa5d00788ca6..f4201748ea297 100644
> --- a/drivers/thermal/qcom/tsens-v1.c
> +++ b/drivers/thermal/qcom/tsens-v1.c
> @@ -145,6 +145,37 @@ static const struct reg_field tsens_v1_regfields[MAX_REGFIELDS] = {
>  	[TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0),
>  };
>  
> +static int __init init_8952(struct tsens_priv *priv)

[Severity: High]
Does marking this function as __init risk a use-after-free if the device is
probed late?

If tsens_probe() runs after the initial kernel boot sequence, for instance due
to a standard -EPROBE_DEFER or a manual sysfs bind, the .init.text section
containing this function will have already been freed. Since this function is
assigned to ops_8952.init and invoked dynamically by tsens_probe(), executing
it after boot would jump into freed memory:

drivers/thermal/qcom/tsens.c:tsens_probe() {
    ...
    if (priv->ops->init) {
        ret = priv->ops->init(priv);
    ...
}

> +{
> +	for (int i = 0; i < priv->num_sensors; ++i)
> +		priv->sensor[i].slope = 3200;
> +
> +	priv->sensor[0].p1_calib_offset = 2;
> +	priv->sensor[0].p2_calib_offset = -1;

[Severity: High]
Could these unconditional writes overflow the priv->sensor flexible array?

In tsens_probe(), the array size is dynamically allocated based on the
"#qcom,sensors" device tree property:

drivers/thermal/qcom/tsens.c:tsens_probe() {
    ...
    of_property_read_u32(np, "#qcom,sensors", &num_sensors);
    ...
    priv = devm_kzalloc(dev,
                 struct_size(priv, sensor, num_sensors),
                 GFP_KERNEL);
    ...
}

If a malformed or custom device tree overrides this property to a value less
than 11, the array will be allocated with fewer elements. However, this code
unconditionally writes to hardcoded indices up to priv->sensor[10], which
could cause a heap buffer overflow.

> +	priv->sensor[1].p1_calib_offset = -4;
> +	priv->sensor[1].p2_calib_offset = -4;
[ ... ]
> +	priv->sensor[10].p1_calib_offset = -3;
> +	priv->sensor[10].p2_calib_offset = -2;
> +
> +	return init_common(priv);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-msm8952-initial-support-v3-0-b96fd3fe298b@mainlining.org?part=16