Re: [PATCH RFC 17/20] thermal: qcom: tsens: Add support for MSM8952 tsens

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Heap Out-Of-Bounds Write in `init_8952` when device tree overrides `#qcom,sensors`.
- [High] Use-After-Free via Section Mismatch: `init_8952` is annotated with `__init`, but can be called after boot memory is freed.
- [Medium] Potential thermal calibration inaccuracy by replacing SoC-specific logic with hardcoded generics.
--

commit 7ec3bedeaa96c98b6f844188546432e42095ad77
Author: Muzaffer Kadir <muzafferkadir@mainlining.org>

thermal: qcom: tsens: Add support for MSM8952 tsens
    
Add MSM8952 support for tsens.
    
Downstream does a lot of advanced things with calculations.
But seems like using tsens-v1 directly works too.

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]
Since the tsens driver probe can be deferred due to nvmem calibration data
dependencies, or the device could be manually unbound and rebound via sysfs,
could this __init annotation cause a use-after-free?

The ops_8952 struct stores the pointer to this function in .rodata. If the
function is freed after boot memory is reclaimed, calling priv->ops->init()
during a deferred probe will jump to a freed address.

[Severity: Medium]
The commit message notes that downstream does advanced calculations, but this
implementation replaces them with static generic offsets. Could this lead to
thermal calibration inaccuracies?

If these static offsets do not accurately reflect the physical thermal
curves of all MSM8952 chips, could the kernel read skewed temperatures
and fail to act on thermal events?

> +{
> +	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;

[ ... ]

> +	priv->sensor[9].p1_calib_offset = 0;
> +	priv->sensor[9].p2_calib_offset = 0;
> +	priv->sensor[10].p1_calib_offset = -3;
> +	priv->sensor[10].p2_calib_offset = -2;

[Severity: High]
Could this result in a heap out-of-bounds write?

In tsens_probe(), num_sensors defaults to 11 but can be overridden by the
"#qcom,sensors" device tree property. The priv->sensor array is then
dynamically allocated based on this count using struct_size().

If a device tree specifies a num_sensors value less than 11, these hardcoded
unconditional assignments will write beyond the bounds of the allocated heap
array.

> +
> +	return init_common(priv);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912-msm8952-initial-support-v1-0-0e742578b524@mainlining.org?part=17