Re: [PATCH v2 2/3] hwmon: ina2xx: support ina232

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Power alert limit calculation always evaluates to 0 on 32-bit systems due to `UINT_MAX` underflow when cast to a signed long.
- [High] Setting a high bus voltage alert limit on 32-bit systems causes signed integer overflow for chips with a high `bus_voltage_shift`.
- [High] Arithmetic overflow and truncation in power and current reading calculations due to 32-bit signed integer limits.
--

commit 1860bedc8e47e2aefaa388ad7ade74649ff17da9
Author: Martino Facchin <m.facchin@arduino.cc>

hwmon: ina2xx: support ina232

The INA232 is a current/power monitor. It shares the same register
layout as the INA2xx and uses the INA226 default configuration, but
differs in its electrical characteristics.

> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index 613ffb622b7c4..fc2319ad99fc3 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
> @@ -196,6 +197,20 @@ static const struct ina2xx_config ina2xx_config[] = {
>  		.current_shift = 4,
>  		.has_update_interval = true,
>  	},
> +	[ina232] = {
> +		.config_default = INA226_CONFIG_DEFAULT,
> +		.calibration_value = 2048,
> +		.shunt_div = 400,
> +		.shunt_voltage_shift = 0,
> +		.bus_voltage_shift = 0,
> +		.bus_voltage_lsb = 1600,
> +		.power_lsb_factor = 32,
> +		.has_alerts = true,
> +		.has_ishunt = false,
> +		.has_power_average = false,
> +		.current_shift = 0,
> +		.has_update_interval = true,
> +	},

[Severity: High]
This is a pre-existing issue, but does power alert configuration work correctly
on 32-bit systems?

In ina226_alert_to_reg() for INA2XX_POWER, the limit is clamped using:

        val = clamp_val(val, 0, UINT_MAX - data->power_lsb_uW);
        val = DIV_ROUND_CLOSEST(val, data->power_lsb_uW);
        return clamp_val(val, 0, USHRT_MAX);

The clamp_val() macro casts its limit arguments to the type of val, which is
a signed long. On 32-bit systems, UINT_MAX - data->power_lsb_uW evaluates to
a negative value.

Because the maximum limit is negative, clamp_val() forces any positive input
to this negative limit, and the final clamp_val(..., 0, USHRT_MAX) will
unconditionally return 0, silently disabling or instantly triggering alerts.

[Severity: High]
This isn't a bug introduced by this patch, but could setting high bus voltage
alerts on 32-bit systems cause an integer overflow?

In ina226_alert_to_reg() for INA2XX_BUS_VOLTAGE, the value is calculated as:

        val = clamp_val(val, 0, 200000);
        val = (val * 1000) << data->config->bus_voltage_shift;

For chips with a high bus_voltage_shift like the INA234 (shift = 4), a valid
high input limit such as 150,000 mV evaluates to (150,000 * 1000) << 4,
which equals 2,400,000,000.

On 32-bit architectures, this exceeds the maximum positive value for a
signed long, overflowing into a negative number. The subsequent clamp to
USHRT_MAX then limits this to 0, converting legitimate high voltage limits
to 0.

[Severity: High]
This is a pre-existing issue, but does this calculation overflow for systems
measuring high loads with low-value shunt resistors?

In ina2xx_get_value(), the calculation is stored in a 32-bit signed integer:

static int ina2xx_get_value(struct ina2xx_data *data, u8 reg,
                            unsigned int regval)
{
        int val;
        ...
        case INA2XX_POWER:
                val = regval * data->power_lsb_uW;
                break;

For a system using a 1 mOhm shunt, power_lsb_uW might be 80,000. If regval
is large (e.g., 65535), the product exceeds INT_MAX.

This overflows on 32-bit systems or truncates when assigned to the 32-bit
integer val on 64-bit systems, causing the driver to report completely
incorrect and often large negative values.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-monza-ina232-v2-0-e4375ce652d0@oss.qualcomm.com?part=2