Re: [PATCH 2/3] hwmon: (pmbus/ltc2978) Add support for LT7175 and LT7176P

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] hwmon: (ltc2978) uninitialized stack read in ltc2978_get_id() for LT7176P
- [Medium] hwmon: (pmbus/ltc2978) IEEE754 peak values compared as LINEAR11 in ltc_get_max()
--

commit 51707b0225138ad0d3a2b0dd58f194120d060f5a
Author: Christopher de Guzman <christopher.deguzman@analog.com>

hwmon: (pmbus/ltc2978) Add support for LT7175 and LT7176P

This commit adds support for the LT7175/LT7175-1 and LT7176P/LT7176P-1 PMBus
buck regulators to the ltc2978 driver, distinguishing them via the
PMBUS_IC_DEVICE_ID string and configuring them with IEEE754 telemetry format.

> diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c
> index aea7b6c93e74b..a42cdc1fe1246 100644
> --- a/drivers/hwmon/pmbus/ltc2978.c
> +++ b/drivers/hwmon/pmbus/ltc2978.c

[ ... ]

> @@ -645,6 +649,10 @@ static int ltc2978_get_id(struct i2c_client *client)
>  			return lt7170;
>  		if (!strncmp(buf, "LT7171", 6))
>  			return lt7171;
> +		if (!strncmp(buf, "LT7175", 6))
> +			return lt7175;
> +		if (!strncmp(buf, "LT7176P", 7))
> +			return lt7176p;

[Severity: Medium]
Can strncmp() read uninitialized stack memory in buf[6] here?

In drivers/hwmon/pmbus/ltc2978.c, ltc2978_get_id() reads PMBUS_IC_DEVICE_ID
into an uninitialized stack buffer via pmbus_read_smbus_i2c_block_data(),
which copies ret bytes without NUL-terminating buf, and then checks only
for ret < 6:

	if (chip_id == LT7170_ID) {
		u8 buf[I2C_SMBUS_BLOCK_MAX];
		int ret;

		ret = pmbus_read_smbus_i2c_block_data(client, PMBUS_IC_DEVICE_ID,
						      buf);
		if (ret < 0)
			return ret;

		if (ret < 6)
			return -ENODEV;

If a device returns a 6-byte PMBUS_IC_DEVICE_ID starting with "LT7176"
(where ret == 6), calling strncmp(buf, "LT7176P", 7) without checking
ret >= 7 will read buf[6] from the uninitialized stack buffer and falsely
match lt7176p if that byte happens to be 'P'.

>  
>  		return -ENODEV;
>  	}
> @@ -774,6 +782,20 @@ static int ltc2978_probe(struct i2c_client *client)
>  		  | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
>  		  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
>  		break;
> +	case lt7175:
> +	case lt7176p:
> +		data->features |= FEAT_CLEAR_PEAKS | FEAT_NEEDS_POLLING;
> +		info->read_word_data = ltc3883_read_word_data;
> +		info->pages = LTC3883_NUM_PAGES;
> +		info->format[PSC_VOLTAGE_IN] = ieee754;
> +		info->format[PSC_VOLTAGE_OUT] = ieee754;
> +		info->format[PSC_CURRENT_OUT] = ieee754;
> +		info->format[PSC_TEMPERATURE] = ieee754;
> +		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
> +		  | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> +		  | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
> +		  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
> +		break;

[Severity: Medium]
Will wiring info->read_word_data to ltc3883_read_word_data cause corrupted
peak readings when the sensor formats are set to ieee754?

For lt7175 and lt7176p, virtual peak reads for PMBUS_VIRT_READ_VIN_MAX,
PMBUS_VIRT_READ_IOUT_MAX, and PMBUS_VIRT_READ_TEMP_MAX go through
ltc3883_read_word_data() -> ltc3880_read_word_data() ->
ltc2978_read_word_data_common() -> ltc_get_max():

static int ltc_get_max(struct ltc2978_data *data, struct i2c_client *client,
		       int page, int reg, u16 *pmax)
{
	int ret;

	ret = ltc_read_word_data(client, page, 0xff, reg);
	if (ret >= 0) {
		if (lin11_to_val(ret) > lin11_to_val(*pmax))
			*pmax = ret;
		ret = *pmax;
	}
	return ret;
}

Because lin11_to_val() treats bit 10 as the sign bit of an 11-bit two's
complement mantissa, whereas in IEEE754 binary16 bit 10 is the least
significant bit of the 5-bit biased exponent, any positive IEEE754 value with
an odd biased exponent (e.g. [1, 2), [4, 8), [16, 32), [64, 128)) has bit 10
set and is decoded by lin11_to_val() as negative.

As a result, when a peak reading increases from an even biased exponent into
an odd biased exponent (such as temperature rising from 40 C to 75 C, or IOUT
rising from 2.5 A to 6.0 A), ltc_get_max() will treat the new higher reading
as negative and fail to update *pmax, or overwrite a higher odd-exponent peak
with a lower even-exponent reading.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020-upstream-lt7175-lt7176p-v1-0-86b9b728b95d@analog.com?part=2