Linux Hardware Monitor development
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Hardware Monitoring <linux-hwmon@vger.kernel.org>
Cc: Guenter Roeck <linux@roeck-us.net>,
	Loic Poulain <loic.poulain@oss.qualcomm.com>
Subject: [PATCH v6] hwmon: (ina2xx) Fix various overflow issues
Date: Sat, 25 Jul 2026 00:58:00 -0700	[thread overview]
Message-ID: <20260725075800.346566-1-linux@roeck-us.net> (raw)

Sashiko reports several integer overflow problems in the ina2xx driver
caused by unbounded multiplications and inadequate types for intermediate
calculations.

Specifically:
- In ina2xx_get_value(), the return type is changed from int to long.
  Intermediate calculations for current are now performed using 64-bit
  types to prevent 32-bit integer overflow before the division by 1000.
- When calculating power in ina2xx_get_value() and
  sy24655_average_power_read(), interim values are cast to u64 and clamped
  to LONG_MAX. This prevents overflow when regval or accumulator_24 is
  multiplied by power_lsb_uW.
- In ina226_alert_to_reg(), the clamping logic is rewritten using min_t().
  This safely avoids integer overflows when scaling user-provided values
  for shunt voltage, bus voltage, power, and current limits.

Cc: Loic Poulain <loic.poulain@oss.qualcomm.com>
Fixes: ab7fbee452be ("hwmon: (ina2xx) Fix various overflow issues")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v6: Replaced some instances of clamp_val() with min_t()
    Simplified commit message
    Fixed even more overflow issues reported by Sashiko in v5
v5: Relaxed clamping when writing the INA2XX_CURRENT alert limit
v4: Fixed another clamping issue when reading INA2XX_CURRENT
v3: Fixed additional problems in INA2XX_CURRENT handling
    (avoid possible 64-bit divide operation and overflow when using
     DIV_ROUND_CLOSEST)
v2: Fixed several additional overflow conditions reported by Sashiko
    after v1

 drivers/hwmon/ina2xx.c | 61 ++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index c4742e84b999..449a72c6b40b 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -16,6 +16,7 @@
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/limits.h>
 #include <linux/module.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
@@ -266,30 +267,34 @@ static u16 ina226_interval_to_reg(long interval)
 	return FIELD_PREP(INA226_AVG_RD_MASK, avg_bits);
 }
 
-static int ina2xx_get_value(struct ina2xx_data *data, u8 reg,
-			    unsigned int regval)
+static long ina2xx_get_value(struct ina2xx_data *data, u8 reg,
+			     unsigned int regval)
 {
-	int val;
+	s64 val64;
+	long val;
 
 	switch (reg) {
 	case INA2XX_SHUNT_VOLTAGE:
 		/* signed register */
-		val = (s16)regval >> data->config->shunt_voltage_shift;
-		val = DIV_ROUND_CLOSEST(val, data->config->shunt_div);
+		val = DIV_ROUND_CLOSEST((s16)regval >> data->config->shunt_voltage_shift,
+					data->config->shunt_div);
 		break;
 	case INA2XX_BUS_VOLTAGE:
-		val = (regval >> data->config->bus_voltage_shift) *
-		  data->config->bus_voltage_lsb;
-		val = DIV_ROUND_CLOSEST(val, 1000);
+		val = DIV_ROUND_CLOSEST((regval >> data->config->bus_voltage_shift) *
+					data->config->bus_voltage_lsb, 1000);
 		break;
 	case INA2XX_POWER:
-		val = regval * data->power_lsb_uW;
+		val = min_t(u64, (u64)regval * data->power_lsb_uW, LONG_MAX);
 		break;
 	case INA2XX_CURRENT:
 		/* signed register, result in mA */
-		val = ((s16)regval >> data->config->current_shift) *
+		val64 = (s64)((s16)regval >> data->config->current_shift) *
 		  data->current_lsb_uA;
-		val = DIV_ROUND_CLOSEST(val, 1000);
+		if (val64 < 0)
+			val64 = -DIV_ROUND_CLOSEST_ULL(-val64, 1000);
+		else
+			val64 = DIV_ROUND_CLOSEST_ULL(val64, 1000);
+		val = clamp_val(val64, LONG_MIN, LONG_MAX);
 		break;
 	case INA2XX_CALIBRATION:
 		val = regval;
@@ -378,27 +383,29 @@ static int ina2xx_read_init(struct device *dev, int reg, long *val)
  */
 static u16 ina226_alert_to_reg(struct ina2xx_data *data, int reg, long val)
 {
+	long limit;
+
 	switch (reg) {
 	case INA2XX_SHUNT_VOLTAGE:
-		val = clamp_val(val, 0, SHRT_MAX * data->config->shunt_div);
-		val *= data->config->shunt_div;
-		val <<= data->config->shunt_voltage_shift;
-		return clamp_val(val, 0, SHRT_MAX);
+		val = min_t(long, val, DIV_ROUND_CLOSEST(SHRT_MAX, data->config->shunt_div));
+		return min_t(long, (val * data->config->shunt_div) << data->config->shunt_voltage_shift,
+			     SHRT_MAX);
 	case INA2XX_BUS_VOLTAGE:
-		val = clamp_val(val, 0, 200000);
-		val = (val * 1000) << data->config->bus_voltage_shift;
-		val = DIV_ROUND_CLOSEST(val, data->config->bus_voltage_lsb);
-		return clamp_val(val, 0, USHRT_MAX);
+		val = min_t(long, val, 130000);
+		return min_t(long,
+			     DIV_ROUND_CLOSEST((val * 1000) << data->config->bus_voltage_shift,
+					       data->config->bus_voltage_lsb),
+			     USHRT_MAX);
 	case INA2XX_POWER:
-		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);
+		val = min_t(long, val, LONG_MAX - data->power_lsb_uW);
+		return min_t(long, DIV_ROUND_CLOSEST(val, data->power_lsb_uW), USHRT_MAX);
 	case INA2XX_CURRENT:
-		val = clamp_val(val, INT_MIN / 1000, INT_MAX / 1000);
+		limit = (LONG_MAX - (data->current_lsb_uA / 2)) / 1000;
+		val = min_t(long, val, limit);
 		/* signed register, result in mA */
 		val = DIV_ROUND_CLOSEST(val * 1000, data->current_lsb_uA);
-		val <<= data->config->current_shift;
-		return clamp_val(val, SHRT_MIN, SHRT_MAX);
+		limit = SHRT_MAX >> data->config->current_shift;
+		return (u16)(min_t(long, val, limit) << data->config->current_shift);
 	default:
 		/* programmer goofed */
 		WARN_ON_ONCE(1);
@@ -537,6 +544,7 @@ static int sy24655_average_power_read(struct ina2xx_data *data, u8 reg, long *va
 	u8 template[6];
 	int ret;
 	long accumulator_24, sample_count;
+	u64 val64;
 
 	/* 48-bit register read */
 	ret = i2c_smbus_read_i2c_block_data(data->client, reg, 6, template);
@@ -555,7 +563,8 @@ static int sy24655_average_power_read(struct ina2xx_data *data, u8 reg, long *va
 		return 0;
 	}
 
-	*val = DIV_ROUND_CLOSEST(accumulator_24, sample_count) * data->power_lsb_uW;
+	val64 = (u64)DIV_ROUND_CLOSEST(accumulator_24, sample_count) * data->power_lsb_uW;
+	*val = min_t(u64, val64, LONG_MAX);
 
 	return 0;
 }
-- 
2.55.0


             reply	other threads:[~2026-07-25  7:58 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-25  7:58 Guenter Roeck [this message]
2026-07-25  8:07 ` [PATCH v6] hwmon: (ina2xx) Fix various overflow issues sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260725075800.346566-1-linux@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=loic.poulain@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox