[PATCH v2 3/9] hwmon: adm1275: Detect coefficient overflow

[Matti Vaittinen]

From: Matti Vaittinen <mazziesaccount@xxxxxxxxx>

Sashiko detected potential coefficient overflow if large shunt resistor
is used. When going unnoticed it can cause "drastically incorrect
telemetry scaling factors" as Sashiko put it.

I am not convinced such "drastically incorrect telemetry scaling
factors" could have gone unnoticed, so I suspect such large shunt
resistors aren't really used. Well, it shouldn't hurt to detect the
error and abort the probe before Really Wrong current / power -values
are reported to user by the hwmon.

Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>

---
Revision history:
v1 => v2:
 - New patch

This patch returns -EOVERFLOW with an error print if overflow is
detected. IF there really are systems where the overflow truly occurs,
then this change will cause the probe to fail - which might hurt the
boot process. It might be safer to only print the warning. One could
also try changing the order of the shunt resistor value division (/1000)
and the multiplication and see if overflow goes away - but it'll be
somewhat more complex then. Hence, I just decided to error-out if this
happens, and leave this for the people facing the real overflow to fix
(if needed)... It's still fair to mention this might cause issues.
---
 drivers/hwmon/pmbus/adm1275.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
index 43baa5ded35e..ccc3ad21e38e 100644
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -839,15 +839,25 @@ static int adm1275_probe(struct i2c_client *client)
 		info->R[PSC_VOLTAGE_OUT] = coefficients[voindex].R;
 	}
 	if (cindex >= 0) {
+		u32 m;
+
 		/* Scale current with sense resistor value */
-		info->m[PSC_CURRENT_OUT] =
-			coefficients[cindex].m * shunt / 1000;
+		if (unlikely(check_mul_overflow(coefficients[cindex].m, shunt, &m))) {
+			dev_err(&client->dev, "Current coefficient overflow\n");
+			return -EOVERFLOW;
+		}
+		info->m[PSC_CURRENT_OUT] = m / 1000;
 		info->b[PSC_CURRENT_OUT] = coefficients[cindex].b;
 		info->R[PSC_CURRENT_OUT] = coefficients[cindex].R;
 	}
 	if (pindex >= 0) {
-		info->m[PSC_POWER] =
-			coefficients[pindex].m * shunt / 1000;
+		u32 m;
+
+		if (unlikely(check_mul_overflow(coefficients[pindex].m, shunt, &m))) {
+			dev_err(&client->dev, "Power coefficient overflow\n");
+			return -EOVERFLOW;
+		}
+		info->m[PSC_POWER] = m / 1000;
 		info->b[PSC_POWER] = coefficients[pindex].b;
 		info->R[PSC_POWER] = coefficients[pindex].R;
 	}
-- 
2.54.0

Attachment: signature.asc
Description: PGP signature