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 <[email protected]> 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-b96fd3fe2...@mainlining.org?part=16
