Re: [PATCH v6 4/5] thermal/drivers/sun8i: Add support for A523 THS0/1 controllers
From: sashiko-bot@kernel.org
Date: 2026-09-13 15:08:04
Also in:
linux-sunxi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect check for unprogrammed eFuse can reject valid calibration data.
- [Medium] Missing bounds check on `callen` in `sun55i_a523_ths_calibrate` allows out-of-bounds memory reads on the dynamically allocated `caldata` array.
--
commit 1a5244bb0d5a03f3ec7f2a8c21b6249015b66057
Author: Mikhail Kalashnikov [off-list ref]
thermal/drivers/sun8i: Add support for A523 THS0/1 controllers
The A523 processor has two temperature controllers, THS0 and THS1.
THS0 has only one temperature sensor, which is located in the DRAM
controller. THS1 does have 4 sensors:
ths1_0 - "big" cores
ths1_1 - "little" cores
ths1_2 - gpu
ths1_3 - npu
quoted hunk ↗ jump to hunk
diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c index 3606548801f02..4a81486f8faec 100644 --- a/drivers/thermal/sun8i_thermal.c +++ b/drivers/thermal/sun8i_thermal.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -298,6 +319,105 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev, return 0; } +/* + * The A523 calibration data is stored in the eFuse, split across two + * nvmem cells. THS1 has four sensors, including ths1_3 which is used + * for the NPU, each with its own calibration value. The fields are + * packed as shown in the layouts below. + * + * efuse layout 0x38-0x3F (caldata[0..3]): + * caldata[0] caldata[1] caldata[2] caldata[3] + * 0 16 24 32 36 48 60 64 + * +---------------+---------------+---------------+---------------+ + * | | | temp | ths1_0 | ths1_1 | + + * +---------------+---------------+---------------+---------------+ + * + * efuse layout 0x44-0x4B (caldata[4..7]): + * caldata[4] caldata[5] caldata[6] caldata[7] + * 0 12 16 24 32 36 48 64 + * +---------------+---------------+---------------+---------------+ + * | ths1_2 | ths1_3 | ths0 | | + + * +---------------+---------------+---------------+---------------+ + */ +static int sun55i_a523_ths_calibrate(struct ths_device *tmdev, + u16 *caldata, int callen) +{ + struct device *dev = tmdev->dev; + int i, ft_temp; + + if (!caldata[1])
[Severity: High] Does this check inadvertently reject valid calibration data? If the 12-bit factory calibration temperature happens to be a multiple of 25.6 C (meaning its lowest 8 bits are 0, like 0x100), caldata[1] will be legitimately 0 since it only holds those lower 8 bits. This would cause the driver to incorrectly abort calibration on perfectly valid hardware. [Severity: Medium] Can this read out-of-bounds memory? The function receives a dynamically allocated caldata array of size callen populated from the NVMEM cells, but callen is never verified here. If the device tree provides a malformed or incomplete NVMEM cell, this access and subsequent reads will happen past the end of the allocation.
+ return -EINVAL;
+
+ ft_temp = (((caldata[2] << 8) | (caldata[1] >> 8)) & FT_TEMP_MASK) * 100;
+
+ for (i = 0; i < tmdev->chip->sensor_num; i++) {
+ int sensor_reg, sensor_temp, cdata, offset;
+ /*
+ * Chips ths0 and ths1 have common parameters for value
+ * calibration. To separate them we can use the number of
+ * temperature sensors on each chip.
+ * For ths0 this value is 1.
+ */
+ if (tmdev->chip->sensor_num == 1) {
+ sensor_reg = ((caldata[5] >> 8) |
+ (caldata[6] << 8)) & TEMP_CALIB_MASK;[Severity: Medium] Is it possible to read out-of-bounds memory here as well? This is the highest index accessed in sun55i_a523_ths_calibrate. It strictly requires a minimum buffer size of 14 bytes. If the device tree provides an 8-byte NVMEM cell, accessing caldata[5] and caldata[6] will pull uninitialized heap data into the calibration calculations or cause a crash.
+ } else {
+ switch (i) {
+ case 0:-- Sashiko AI review · https://sashiko.dev/#/patchset/20260913145527.18148-1-iuncuim@gmail.com?part=4