* [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko
@ 2026-08-05 0:57 Guenter Roeck
2026-08-05 0:57 ` [PATCH 1/3] hwmon: (ltc4282) Avoid overflow in maximum power calculation Guenter Roeck
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Guenter Roeck @ 2026-08-05 0:57 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Nuno Sa, Guenter Roeck
This series started with a patch fixing a potential overflow issue in
maximum poer calculations. Sashiko then reported a similar problem when
setting current limits, and finally found a problem when evaluating
adi,current-limit-sense-microvolt.
This series fixes all those problems.
Note that Sashiko reports more issues in this driver. Leave those
to be fixed later.
----------------------------------------------------------------
Guenter Roeck (3):
hwmon: (ltc4282) Avoid overflow in maximum power calculation
hwmon: (ltc4282) Clamp negative current limits
hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt
drivers/hwmon/ltc4282.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] hwmon: (ltc4282) Avoid overflow in maximum power calculation
2026-08-05 0:57 [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko Guenter Roeck
@ 2026-08-05 0:57 ` Guenter Roeck
2026-08-05 1:07 ` sashiko-bot
2026-08-05 0:57 ` [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits Guenter Roeck
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2026-08-05 0:57 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Nuno Sa, Guenter Roeck, Sashiko
During device initialization in ltc4282_set_max_limits(), the calculation
of the maximum power limit can suffer from a 32-bit integer overflow.
static int ltc4282_set_max_limits(struct ltc4282_state *st)
{
...
st->power_max = DIV_ROUND_CLOSEST(st->vsense_max * DECA * MILLI,
st->rsense) * st->vfs_out;
...
}
The result of DIV_ROUND_CLOSEST() evaluates to a 32-bit unsigned integer
on 32-bit architectures. This result is then multiplied by st->vfs_out,
which is a 16-bit unsigned integer. According to C promotion rules, since
both operands are 32-bit or smaller, the multiplication is performed in
32-bit precision.
If the device is configured with a low sense resistor value via the device
tree (for example, 100 nano-ohms, resulting in st->rsense = 1) and the
voltage is high, the division result can reach 343,750,000 and st->vfs_out
can be 33,280. The product of these values is approximately 11.44 trillion,
which exceeds the maximum capacity of a 32-bit integer and overflows
before being stored in st->power_max.
This overflow causes a truncated value to be assigned to st->power_max and
written to the hardware limit register. An incorrect maximum power limit
can trigger spurious power-bad faults or alarms, which may lead to the
shutdown of the monitored power rail.
Avoid the problem by calculating and storing the maximum power using 64-bit
variables.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: cbc29538dbf7d ("hwmon: Add driver for LTC4282")
Cc: Nuno Sa <nuno.sa@analog.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/ltc4282.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
index cc698803f8bf..bb7f6727c44d 100644
--- a/drivers/hwmon/ltc4282.c
+++ b/drivers/hwmon/ltc4282.c
@@ -137,7 +137,7 @@ struct ltc4282_state {
*/
struct ltc4282_cache in0_1_cache[LTC4282_CHAN_VGPIO];
u32 vsense_max;
- long power_max;
+ s64 power_max;
u32 rsense;
u16 vdd;
u16 vfs_out;
@@ -613,13 +613,12 @@ static int ltc4282_read(struct device *dev, enum hwmon_sensor_types type,
}
static int ltc4282_write_power_byte(const struct ltc4282_state *st, u32 reg,
- long val)
+ s64 val)
{
u32 power;
u64 temp;
- if (val > st->power_max)
- val = st->power_max;
+ val = clamp(val, 0, st->power_max);
temp = val * int_pow(U8_MAX, 2) * st->rsense;
power = DIV64_U64_ROUND_CLOSEST(temp,
@@ -629,7 +628,7 @@ static int ltc4282_write_power_byte(const struct ltc4282_state *st, u32 reg,
}
static int ltc4282_write_power_word(const struct ltc4282_state *st, u32 reg,
- long val)
+ u64 val)
{
u64 temp = int_pow(U16_MAX, 2) * st->rsense, temp_2;
__be16 __raw;
@@ -1222,7 +1221,8 @@ static int ltc4282_set_max_limits(struct ltc4282_state *st)
return ret;
/* Power is given by ISENSE * Vout. */
- st->power_max = DIV_ROUND_CLOSEST(st->vsense_max * DECA * MILLI, st->rsense) * st->vfs_out;
+ st->power_max = DIV_ROUND_CLOSEST_ULL((u64)st->vsense_max * DECA * MILLI,
+ st->rsense) * st->vfs_out;
ret = ltc4282_write_power_byte(st, LTC4282_POWER_MAX, st->power_max);
if (ret)
return ret;
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits
2026-08-05 0:57 [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko Guenter Roeck
2026-08-05 0:57 ` [PATCH 1/3] hwmon: (ltc4282) Avoid overflow in maximum power calculation Guenter Roeck
@ 2026-08-05 0:57 ` Guenter Roeck
2026-08-05 1:04 ` sashiko-bot
2026-08-05 9:22 ` Nuno Sá
2026-08-05 0:57 ` [PATCH 3/3] hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt Guenter Roeck
2026-08-05 9:26 ` [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko Nuno Sá
3 siblings, 2 replies; 14+ messages in thread
From: Guenter Roeck @ 2026-08-05 0:57 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Nuno Sa, Guenter Roeck, Sashiko
When a negative value is passed to ltc4282_write_curr(), the signed long
val is cast directly to u64:
drivers/hwmon/ltc4282.c:ltc4282_write_curr() {
/* need to pass it in millivolt */
u32 in = DIV_ROUND_CLOSEST_ULL((u64)val * st->rsense, DECA * MICRO);
...
}
This cast converts negative inputs into large positive values. The
subsequent division result overflows the u32 in variable, truncating
to a pseudo-random positive value. When this is passed to
ltc4282_write_voltage_byte(), it is clamped to the maximum limit instead
of zero.
Clamp val to 0 and to the maximum supported upper limit before the cast
and assign the result to a 64-bit temporary variable before the division
to avoid the underflow and an also possible overflow.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: cbc29538dbf7d ("hwmon: Add driver for LTC4282")
Cc: Nuno Sa <nuno.sa@analog.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/ltc4282.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
index bb7f6727c44d..bb1bcb369016 100644
--- a/drivers/hwmon/ltc4282.c
+++ b/drivers/hwmon/ltc4282.c
@@ -14,6 +14,7 @@
#include <linux/hwmon.h>
#include <linux/i2c.h>
#include <linux/math.h>
+#include <linux/math64.h>
#include <linux/minmax.h>
#include <linux/module.h>
#include <linux/regmap.h>
@@ -929,8 +930,11 @@ static int ltc4282_curr_reset_hist(struct ltc4282_state *st)
static int ltc4282_write_curr(struct ltc4282_state *st, u32 attr,
long val)
{
+ s32 ulimit = min_t(u64, INT_MAX,
+ div_u64((u64)INT_MAX * DECA * MICRO, st->rsense));
+ u64 val64 = clamp(val, 0, ulimit);
/* need to pass it in millivolt */
- u32 in = DIV_ROUND_CLOSEST_ULL((u64)val * st->rsense, DECA * MICRO);
+ u32 in = DIV_ROUND_CLOSEST_ULL(val64 * st->rsense, DECA * MICRO);
switch (attr) {
case hwmon_curr_max:
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt
2026-08-05 0:57 [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko Guenter Roeck
2026-08-05 0:57 ` [PATCH 1/3] hwmon: (ltc4282) Avoid overflow in maximum power calculation Guenter Roeck
2026-08-05 0:57 ` [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits Guenter Roeck
@ 2026-08-05 0:57 ` Guenter Roeck
2026-08-05 1:08 ` sashiko-bot
2026-08-05 9:26 ` [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko Nuno Sá
3 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2026-08-05 0:57 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Nuno Sa, Guenter Roeck, Sashiko
ltc4282_parse_dt() evaluates the wrong variable when parsing the current
limit.
When the adi,current-limit-sense-microvolt property is parsed into
st->vsense_max, the subsequent switch statement evaluates the unrelated
val variable instead of st->vsense_max:
drivers/hwmon/ltc4282.c:ltc4282_parse_dt() {
...
ret = device_property_read_u32(dev, "adi,current-limit-sense-microvolt",
&st->vsense_max);
if (!ret) {
int reg_val;
switch (val) {
case 12500:
reg_val = 0;
break;
...
}
Because val holds a small integer representing vin_mode (from 0 to 3), it
never matches any of the valid current limit cases.
This causes it to always fall through to the default error case, return
-EINVAL, and aborts probe initialization for any device tree using this
property.
Validate st->vsense_max instead to fix the problem.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: cbc29538dbf7d ("hwmon: Add driver for LTC4282")
Cc: Nuno Sa <nuno.sa@analog.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/ltc4282.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
index bb1bcb369016..b1675dc5b3c7 100644
--- a/drivers/hwmon/ltc4282.c
+++ b/drivers/hwmon/ltc4282.c
@@ -1394,7 +1394,7 @@ static int ltc4282_setup(struct ltc4282_state *st, struct device *dev)
if (!ret) {
int reg_val;
- switch (val) {
+ switch (st->vsense_max) {
case 12500:
reg_val = 0;
break;
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits
2026-08-05 0:57 ` [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits Guenter Roeck
@ 2026-08-05 1:04 ` sashiko-bot
2026-08-05 9:22 ` Nuno Sá
1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-05 1:04 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-hwmon
> When a negative value is passed to ltc4282_write_curr(), the signed long
> val is cast directly to u64:
>
> drivers/hwmon/ltc4282.c:ltc4282_write_curr() {
> /* need to pass it in millivolt */
> u32 in = DIV_ROUND_CLOSEST_ULL((u64)val * st->rsense, DECA * MICRO);
> ...
> }
>
> This cast converts negative inputs into large positive values. The
> subsequent division result overflows the u32 in variable, truncating
> to a pseudo-random positive value. When this is passed to
> ltc4282_write_voltage_byte(), it is clamped to the maximum limit instead
> of zero.
>
> Clamp val to 0 and to the maximum supported upper limit before the cast
> and assign the result to a 64-bit temporary variable before the division
> to avoid the underflow and an also possible overflow.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: cbc29538dbf7d ("hwmon: Add driver for LTC4282")
> Cc: Nuno Sa <nuno.sa@analog.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805005721.1566034-1-linux@roeck-us.net?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] hwmon: (ltc4282) Avoid overflow in maximum power calculation
2026-08-05 0:57 ` [PATCH 1/3] hwmon: (ltc4282) Avoid overflow in maximum power calculation Guenter Roeck
@ 2026-08-05 1:07 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-05 1:07 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-hwmon
> During device initialization in ltc4282_set_max_limits(), the calculation
> of the maximum power limit can suffer from a 32-bit integer overflow.
>
> static int ltc4282_set_max_limits(struct ltc4282_state *st)
> {
> ...
> st->power_max = DIV_ROUND_CLOSEST(st->vsense_max * DECA * MILLI,
> st->rsense) * st->vfs_out;
> ...
> }
>
> The result of DIV_ROUND_CLOSEST() evaluates to a 32-bit unsigned integer
> on 32-bit architectures. This result is then multiplied by st->vfs_out,
> which is a 16-bit unsigned integer. According to C promotion rules, since
> both operands are 32-bit or smaller, the multiplication is performed in
> [ ... ]
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: cbc29538dbf7d ("hwmon: Add driver for LTC4282")
> Cc: Nuno Sa <nuno.sa@analog.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805005721.1566034-1-linux@roeck-us.net?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt
2026-08-05 0:57 ` [PATCH 3/3] hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt Guenter Roeck
@ 2026-08-05 1:08 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-05 1:08 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-hwmon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The `in_min_raw` cache for the inactive voltage monitoring channel is not initialized from the hardware EEPROM at boot, leading to the hardware undervoltage limit being clobbered with 0 when the channel is enabled.
- [High] Uninitialized stack variable `struct clk_init_data init` is passed to the Common Clock Framework, leading to undefined behavior and potential panics.
--
--- Patch [3]: [PATCH 3/3] hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt ---
commit b52e0acebe8052dfb365a719ef77509a10b27e3a
Author: Guenter Roeck <linux@roeck-us.net>
hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt
ltc4282_parse_dt() evaluates the wrong variable when parsing the current
limit.
> diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
> index bb1bcb3690163..b1675dc5b3c7f 100644
> --- a/drivers/hwmon/ltc4282.c
> +++ b/drivers/hwmon/ltc4282.c
> @@ -1394,7 +1394,7 @@ static int ltc4282_setup(struct ltc4282_state *st, struct device *dev)
> if (!ret) {
> int reg_val;
>
> - switch (val) {
> + switch (st->vsense_max) {
> case 12500:
> reg_val = 0;
> break;
[Severity: High]
This isn't a bug introduced by this patch, but the in_min_raw cache for the
inactive voltage monitoring channel does not appear to be initialized from
hardware defaults during boot:
drivers/hwmon/ltc4282.c:ltc4282_get_defaults() {
...
if (!st->in0_1_cache[LTC4282_CHAN_VSOURCE].en) {
st->in0_1_cache[LTC4282_CHAN_VDD].en = true;
return regmap_read(st->map, LTC4282_VSOURCE_MAX,
&st->in0_1_cache[LTC4282_CHAN_VSOURCE].in_max_raw);
}
return regmap_read(st->map, LTC4282_VSOURCE_MAX,
&st->in0_1_cache[LTC4282_CHAN_VDD].in_max_raw);
}
If a user later enables this inactive channel via sysfs, ltc4282_cache_sync()
writes the uninitialized in_min_raw (0) back to the hardware register:
drivers/hwmon/ltc4282.c:ltc4282_cache_sync() {
...
ret = regmap_write(st->map, LTC4282_VSOURCE_MIN,
st->in0_1_cache[channel].in_min_raw);
...
}
Does this silently clobber and disable the hardware undervoltage limit?
[Severity: High]
This is a pre-existing issue, but I also noticed a potential problem with
uninitialized stack variables passed to the common clock framework:
drivers/hwmon/ltc4282.c:ltc428_clk_provider_setup() {
struct clk_init_data init;
int ret;
if (!IS_ENABLED(CONFIG_COMMON_CLK))
return 0;
init.name = devm_kasprintf(dev, GFP_KERNEL, "%s-clk",
fwnode_get_name(dev_fwnode(dev)));
...
init.ops = <c4282_ops;
...
}
Since init is not zero-initialized, fields like init.num_parents and
init.parent_names will contain stack garbage. When devm_clk_hw_register()
evaluates init.num_parents, could it attempt to dereference the uninitialized
init.parent_names pointer and cause a panic during driver probe?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805005721.1566034-1-linux@roeck-us.net?part=3
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits
2026-08-05 0:57 ` [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits Guenter Roeck
2026-08-05 1:04 ` sashiko-bot
@ 2026-08-05 9:22 ` Nuno Sá
2026-08-05 15:41 ` Guenter Roeck
1 sibling, 1 reply; 14+ messages in thread
From: Nuno Sá @ 2026-08-05 9:22 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hardware Monitoring, Nuno Sa, Sashiko
On Tue, Aug 04, 2026 at 05:57:20PM -0700, Guenter Roeck wrote:
> When a negative value is passed to ltc4282_write_curr(), the signed long
> val is cast directly to u64:
>
> drivers/hwmon/ltc4282.c:ltc4282_write_curr() {
> /* need to pass it in millivolt */
> u32 in = DIV_ROUND_CLOSEST_ULL((u64)val * st->rsense, DECA * MICRO);
> ...
> }
>
> This cast converts negative inputs into large positive values. The
> subsequent division result overflows the u32 in variable, truncating
> to a pseudo-random positive value. When this is passed to
> ltc4282_write_voltage_byte(), it is clamped to the maximum limit instead
> of zero.
>
> Clamp val to 0 and to the maximum supported upper limit before the cast
> and assign the result to a 64-bit temporary variable before the division
> to avoid the underflow and an also possible overflow.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: cbc29538dbf7d ("hwmon: Add driver for LTC4282")
> Cc: Nuno Sa <nuno.sa@analog.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/hwmon/ltc4282.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
> index bb7f6727c44d..bb1bcb369016 100644
> --- a/drivers/hwmon/ltc4282.c
> +++ b/drivers/hwmon/ltc4282.c
> @@ -14,6 +14,7 @@
> #include <linux/hwmon.h>
> #include <linux/i2c.h>
> #include <linux/math.h>
> +#include <linux/math64.h>
> #include <linux/minmax.h>
> #include <linux/module.h>
> #include <linux/regmap.h>
> @@ -929,8 +930,11 @@ static int ltc4282_curr_reset_hist(struct ltc4282_state *st)
> static int ltc4282_write_curr(struct ltc4282_state *st, u32 attr,
> long val)
> {
> + s32 ulimit = min_t(u64, INT_MAX,
> + div_u64((u64)INT_MAX * DECA * MICRO, st->rsense));
I guess we can do the same as in ltc4283 instead of just assuming
INT_MAX:
https://elixir.bootlin.com/linux/v7.2-rc5/source/drivers/hwmon/ltc4283.c#L765
And so we just account for isense_max
- Nuno Sá
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko
2026-08-05 0:57 [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko Guenter Roeck
` (2 preceding siblings ...)
2026-08-05 0:57 ` [PATCH 3/3] hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt Guenter Roeck
@ 2026-08-05 9:26 ` Nuno Sá
2026-08-05 14:28 ` Guenter Roeck
3 siblings, 1 reply; 14+ messages in thread
From: Nuno Sá @ 2026-08-05 9:26 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hardware Monitoring, Nuno Sa
On Tue, Aug 04, 2026 at 05:57:18PM -0700, Guenter Roeck wrote:
> This series started with a patch fixing a potential overflow issue in
> maximum poer calculations. Sashiko then reported a similar problem when
> setting current limits, and finally found a problem when evaluating
> adi,current-limit-sense-microvolt.
>
> This series fixes all those problems.
>
> Note that Sashiko reports more issues in this driver. Leave those
> to be fixed later.
>
> ----------------------------------------------------------------
> Guenter Roeck (3):
> hwmon: (ltc4282) Avoid overflow in maximum power calculation
> hwmon: (ltc4282) Clamp negative current limits
> hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt
>
> drivers/hwmon/ltc4282.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
Hi Guenter,
During ltc4283 sashiko's review was clear to me that this one also needed some
attention but no time for it so far. So thanks for this.
Also, the clk_init_data remark is very much an issue. Not sure if you
can fit that in this series. Or I can also send a quick patch for it.
Regarding the series, just one suggestion from me. With that:
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko
2026-08-05 9:26 ` [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko Nuno Sá
@ 2026-08-05 14:28 ` Guenter Roeck
2026-08-05 16:15 ` Nuno Sá
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2026-08-05 14:28 UTC (permalink / raw)
To: Nuno Sá; +Cc: Hardware Monitoring, Nuno Sa
On 8/5/26 02:26, Nuno Sá wrote:
> On Tue, Aug 04, 2026 at 05:57:18PM -0700, Guenter Roeck wrote:
>> This series started with a patch fixing a potential overflow issue in
>> maximum poer calculations. Sashiko then reported a similar problem when
>> setting current limits, and finally found a problem when evaluating
>> adi,current-limit-sense-microvolt.
>>
>> This series fixes all those problems.
>>
>> Note that Sashiko reports more issues in this driver. Leave those
>> to be fixed later.
>>
>> ----------------------------------------------------------------
>> Guenter Roeck (3):
>> hwmon: (ltc4282) Avoid overflow in maximum power calculation
>> hwmon: (ltc4282) Clamp negative current limits
>> hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt
>>
>> drivers/hwmon/ltc4282.c | 20 ++++++++++++--------
>> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> Hi Guenter,
>
> During ltc4283 sashiko's review was clear to me that this one also needed some
> attention but no time for it so far. So thanks for this.
>
> Also, the clk_init_data remark is very much an issue. Not sure if you
> can fit that in this series. Or I can also send a quick patch for it.
>
I did run the series through Sashiko several times before I sent it out, and
it did report a variety of other issues, but not this one. I am not really sure
if initializing the data structure is sufficient, so I would appreciate if you
would send a patch.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits
2026-08-05 9:22 ` Nuno Sá
@ 2026-08-05 15:41 ` Guenter Roeck
2026-08-05 16:14 ` Nuno Sá
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2026-08-05 15:41 UTC (permalink / raw)
To: Nuno Sá; +Cc: Hardware Monitoring, Nuno Sa, Sashiko
On 8/5/26 02:22, Nuno Sá wrote:
> On Tue, Aug 04, 2026 at 05:57:20PM -0700, Guenter Roeck wrote:
>> When a negative value is passed to ltc4282_write_curr(), the signed long
>> val is cast directly to u64:
>>
>> drivers/hwmon/ltc4282.c:ltc4282_write_curr() {
>> /* need to pass it in millivolt */
>> u32 in = DIV_ROUND_CLOSEST_ULL((u64)val * st->rsense, DECA * MICRO);
>> ...
>> }
>>
>> This cast converts negative inputs into large positive values. The
>> subsequent division result overflows the u32 in variable, truncating
>> to a pseudo-random positive value. When this is passed to
>> ltc4282_write_voltage_byte(), it is clamped to the maximum limit instead
>> of zero.
>>
>> Clamp val to 0 and to the maximum supported upper limit before the cast
>> and assign the result to a 64-bit temporary variable before the division
>> to avoid the underflow and an also possible overflow.
>>
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Fixes: cbc29538dbf7d ("hwmon: Add driver for LTC4282")
>> Cc: Nuno Sa <nuno.sa@analog.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> drivers/hwmon/ltc4282.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
>> index bb7f6727c44d..bb1bcb369016 100644
>> --- a/drivers/hwmon/ltc4282.c
>> +++ b/drivers/hwmon/ltc4282.c
>> @@ -14,6 +14,7 @@
>> #include <linux/hwmon.h>
>> #include <linux/i2c.h>
>> #include <linux/math.h>
>> +#include <linux/math64.h>
>> #include <linux/minmax.h>
>> #include <linux/module.h>
>> #include <linux/regmap.h>
>> @@ -929,8 +930,11 @@ static int ltc4282_curr_reset_hist(struct ltc4282_state *st)
>> static int ltc4282_write_curr(struct ltc4282_state *st, u32 attr,
>> long val)
>> {
>> + s32 ulimit = min_t(u64, INT_MAX,
>> + div_u64((u64)INT_MAX * DECA * MICRO, st->rsense));
>
> I guess we can do the same as in ltc4283 instead of just assuming
> INT_MAX:
>
> https://elixir.bootlin.com/linux/v7.2-rc5/source/drivers/hwmon/ltc4283.c#L765
>
> And so we just account for isense_max
>
Sashiko isn't happy with it. It suggests the multiplier in the limit calculation
should use MILLI, not MICRO, and that a 32-bit operation could overflow.
With that, I'd rather keep it as is, or drop that patch and let you handle it.
Please let me know what you prefer.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits
2026-08-05 15:41 ` Guenter Roeck
@ 2026-08-05 16:14 ` Nuno Sá
2026-08-05 17:19 ` Guenter Roeck
0 siblings, 1 reply; 14+ messages in thread
From: Nuno Sá @ 2026-08-05 16:14 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hardware Monitoring, Nuno Sa, Sashiko
On Wed, Aug 05, 2026 at 08:41:30AM -0700, Guenter Roeck wrote:
> On 8/5/26 02:22, Nuno Sá wrote:
> > On Tue, Aug 04, 2026 at 05:57:20PM -0700, Guenter Roeck wrote:
> > > When a negative value is passed to ltc4282_write_curr(), the signed long
> > > val is cast directly to u64:
> > >
> > > drivers/hwmon/ltc4282.c:ltc4282_write_curr() {
> > > /* need to pass it in millivolt */
> > > u32 in = DIV_ROUND_CLOSEST_ULL((u64)val * st->rsense, DECA * MICRO);
> > > ...
> > > }
> > >
> > > This cast converts negative inputs into large positive values. The
> > > subsequent division result overflows the u32 in variable, truncating
> > > to a pseudo-random positive value. When this is passed to
> > > ltc4282_write_voltage_byte(), it is clamped to the maximum limit instead
> > > of zero.
> > >
> > > Clamp val to 0 and to the maximum supported upper limit before the cast
> > > and assign the result to a 64-bit temporary variable before the division
> > > to avoid the underflow and an also possible overflow.
> > >
> > > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > > Fixes: cbc29538dbf7d ("hwmon: Add driver for LTC4282")
> > > Cc: Nuno Sa <nuno.sa@analog.com>
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > ---
> > > drivers/hwmon/ltc4282.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
> > > index bb7f6727c44d..bb1bcb369016 100644
> > > --- a/drivers/hwmon/ltc4282.c
> > > +++ b/drivers/hwmon/ltc4282.c
> > > @@ -14,6 +14,7 @@
> > > #include <linux/hwmon.h>
> > > #include <linux/i2c.h>
> > > #include <linux/math.h>
> > > +#include <linux/math64.h>
> > > #include <linux/minmax.h>
> > > #include <linux/module.h>
> > > #include <linux/regmap.h>
> > > @@ -929,8 +930,11 @@ static int ltc4282_curr_reset_hist(struct ltc4282_state *st)
> > > static int ltc4282_write_curr(struct ltc4282_state *st, u32 attr,
> > > long val)
> > > {
> > > + s32 ulimit = min_t(u64, INT_MAX,
> > > + div_u64((u64)INT_MAX * DECA * MICRO, st->rsense));
> >
> > I guess we can do the same as in ltc4283 instead of just assuming
> > INT_MAX:
> >
> > https://elixir.bootlin.com/linux/v7.2-rc5/source/drivers/hwmon/ltc4283.c#L765
> >
> > And so we just account for isense_max
> >
>
> Sashiko isn't happy with it. It suggests the multiplier in the limit calculation
> should use MILLI, not MICRO, and that a 32-bit operation could overflow.
> With that, I'd rather keep it as is, or drop that patch and let you handle it.
> Please let me know what you prefer.
>
I guess we would need to also do what we have in ltc4283 and limit the
range of values rsense is allowed to have so we can better reason on
what can or cannot overflow. But I can't really commit to when I can
handle this so I'm fine with the above approach.
- Nuno Sá
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko
2026-08-05 14:28 ` Guenter Roeck
@ 2026-08-05 16:15 ` Nuno Sá
0 siblings, 0 replies; 14+ messages in thread
From: Nuno Sá @ 2026-08-05 16:15 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hardware Monitoring, Nuno Sa
On Wed, Aug 05, 2026 at 07:28:34AM -0700, Guenter Roeck wrote:
> On 8/5/26 02:26, Nuno Sá wrote:
> > On Tue, Aug 04, 2026 at 05:57:18PM -0700, Guenter Roeck wrote:
> > > This series started with a patch fixing a potential overflow issue in
> > > maximum poer calculations. Sashiko then reported a similar problem when
> > > setting current limits, and finally found a problem when evaluating
> > > adi,current-limit-sense-microvolt.
> > >
> > > This series fixes all those problems.
> > >
> > > Note that Sashiko reports more issues in this driver. Leave those
> > > to be fixed later.
> > >
> > > ----------------------------------------------------------------
> > > Guenter Roeck (3):
> > > hwmon: (ltc4282) Avoid overflow in maximum power calculation
> > > hwmon: (ltc4282) Clamp negative current limits
> > > hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt
> > >
> > > drivers/hwmon/ltc4282.c | 20 ++++++++++++--------
> > > 1 file changed, 12 insertions(+), 8 deletions(-)
> >
> > Hi Guenter,
> >
> > During ltc4283 sashiko's review was clear to me that this one also needed some
> > attention but no time for it so far. So thanks for this.
> >
> > Also, the clk_init_data remark is very much an issue. Not sure if you
> > can fit that in this series. Or I can also send a quick patch for it.
> >
>
> I did run the series through Sashiko several times before I sent it out, and
> it did report a variety of other issues, but not this one. I am not really sure
> if initializing the data structure is sufficient, so I would appreciate if you
> would send a patch.
My plan it's really to just add ` = { };`. At least for the clock flags
might really be problematic to have some random value.
- Nuno Sá
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits
2026-08-05 16:14 ` Nuno Sá
@ 2026-08-05 17:19 ` Guenter Roeck
0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2026-08-05 17:19 UTC (permalink / raw)
To: Nuno Sá; +Cc: Hardware Monitoring, Nuno Sa, Sashiko
On 8/5/26 09:14, Nuno Sá wrote:
> On Wed, Aug 05, 2026 at 08:41:30AM -0700, Guenter Roeck wrote:
>> On 8/5/26 02:22, Nuno Sá wrote:
>>> On Tue, Aug 04, 2026 at 05:57:20PM -0700, Guenter Roeck wrote:
>>>> When a negative value is passed to ltc4282_write_curr(), the signed long
>>>> val is cast directly to u64:
>>>>
>>>> drivers/hwmon/ltc4282.c:ltc4282_write_curr() {
>>>> /* need to pass it in millivolt */
>>>> u32 in = DIV_ROUND_CLOSEST_ULL((u64)val * st->rsense, DECA * MICRO);
>>>> ...
>>>> }
>>>>
>>>> This cast converts negative inputs into large positive values. The
>>>> subsequent division result overflows the u32 in variable, truncating
>>>> to a pseudo-random positive value. When this is passed to
>>>> ltc4282_write_voltage_byte(), it is clamped to the maximum limit instead
>>>> of zero.
>>>>
>>>> Clamp val to 0 and to the maximum supported upper limit before the cast
>>>> and assign the result to a 64-bit temporary variable before the division
>>>> to avoid the underflow and an also possible overflow.
>>>>
>>>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>>>> Fixes: cbc29538dbf7d ("hwmon: Add driver for LTC4282")
>>>> Cc: Nuno Sa <nuno.sa@analog.com>
>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>> ---
>>>> drivers/hwmon/ltc4282.c | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
>>>> index bb7f6727c44d..bb1bcb369016 100644
>>>> --- a/drivers/hwmon/ltc4282.c
>>>> +++ b/drivers/hwmon/ltc4282.c
>>>> @@ -14,6 +14,7 @@
>>>> #include <linux/hwmon.h>
>>>> #include <linux/i2c.h>
>>>> #include <linux/math.h>
>>>> +#include <linux/math64.h>
>>>> #include <linux/minmax.h>
>>>> #include <linux/module.h>
>>>> #include <linux/regmap.h>
>>>> @@ -929,8 +930,11 @@ static int ltc4282_curr_reset_hist(struct ltc4282_state *st)
>>>> static int ltc4282_write_curr(struct ltc4282_state *st, u32 attr,
>>>> long val)
>>>> {
>>>> + s32 ulimit = min_t(u64, INT_MAX,
>>>> + div_u64((u64)INT_MAX * DECA * MICRO, st->rsense));
>>>
>>> I guess we can do the same as in ltc4283 instead of just assuming
>>> INT_MAX:
>>>
>>> https://elixir.bootlin.com/linux/v7.2-rc5/source/drivers/hwmon/ltc4283.c#L765
>>>
>>> And so we just account for isense_max
>>>
>>
>> Sashiko isn't happy with it. It suggests the multiplier in the limit calculation
>> should use MILLI, not MICRO, and that a 32-bit operation could overflow.
>> With that, I'd rather keep it as is, or drop that patch and let you handle it.
>> Please let me know what you prefer.
>>
>
> I guess we would need to also do what we have in ltc4283 and limit the
> range of values rsense is allowed to have so we can better reason on
> what can or cannot overflow. But I can't really commit to when I can
> handle this so I'm fine with the above approach.
>
Makes sense. I'll apply the series as-is. If you (or me) ever get to
it we can always improve on it.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-08-05 17:19 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 0:57 [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko Guenter Roeck
2026-08-05 0:57 ` [PATCH 1/3] hwmon: (ltc4282) Avoid overflow in maximum power calculation Guenter Roeck
2026-08-05 1:07 ` sashiko-bot
2026-08-05 0:57 ` [PATCH 2/3] hwmon: (ltc4282) Clamp negative current limits Guenter Roeck
2026-08-05 1:04 ` sashiko-bot
2026-08-05 9:22 ` Nuno Sá
2026-08-05 15:41 ` Guenter Roeck
2026-08-05 16:14 ` Nuno Sá
2026-08-05 17:19 ` Guenter Roeck
2026-08-05 0:57 ` [PATCH 3/3] hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt Guenter Roeck
2026-08-05 1:08 ` sashiko-bot
2026-08-05 9:26 ` [PATCH 0/3] hwmon: (ltc4282) Fix issues reported by Sashiko Nuno Sá
2026-08-05 14:28 ` Guenter Roeck
2026-08-05 16:15 ` Nuno Sá
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox