Skip to content

Commit ab7fbee

Browse files
committed
hwmon: (ina2xx) Fix various overflow issues
Module tests show various overflow problems when writing limits and other attributes. in0_crit: Suspected overflow: [max=82, read 0, written 2147483648] in0_lcrit: Suspected overflow: [max=82, read 0, written 2147483648] in1_crit: Suspected overflow: [max=40959, read 0, written 2147483647] in1_lcrit: Suspected overflow: [max=40959, read 0, written 2147483647] power1_crit: Suspected overflow: [max=134218750, read 0, written 2147483648] update_interval: Suspected overflow: [max=2253, read 2, written 2147483647] Implement missing clamping on attribute write operations to avoid those problems. While at it, check in the probe function if the shunt resistor value passed from devicetree is valid, and bail out if it isn't. Also limit mutex use to the code calling ina2xx_set_shunt() since it isn't needed when called from the probe function. Reviewed-by: Tzung-Bi Shih <[email protected]> Signed-off-by: Guenter Roeck <[email protected]>
1 parent bb25cdc commit ab7fbee

File tree

1 file changed

+20
-12
lines changed

1 file changed

+20
-12
lines changed

drivers/hwmon/ina2xx.c

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,16 @@ static int ina226_reg_to_interval(u16 config)
193193
* Return the new, shifted AVG field value of CONFIG register,
194194
* to use with regmap_update_bits
195195
*/
196-
static u16 ina226_interval_to_reg(int interval)
196+
static u16 ina226_interval_to_reg(unsigned long interval)
197197
{
198198
int avg, avg_bits;
199199

200+
/*
201+
* The maximum supported interval is 1,024 * (2 * 8.244ms) ~= 16.8s.
202+
* Clamp to 32 seconds before calculations to avoid overflows.
203+
*/
204+
interval = clamp_val(interval, 0, 32000);
205+
200206
avg = DIV_ROUND_CLOSEST(interval * 1000,
201207
INA226_TOTAL_CONV_TIME_DEFAULT);
202208
avg_bits = find_closest(avg, ina226_avg_tab,
@@ -371,19 +377,22 @@ static int ina226_reg_to_alert(struct ina2xx_data *data, u32 mask, u16 regval)
371377
* Turns alert limit values into register values.
372378
* Opposite of the formula in ina2xx_get_value().
373379
*/
374-
static u16 ina226_alert_to_reg(struct ina2xx_data *data, u32 mask, int val)
380+
static u16 ina226_alert_to_reg(struct ina2xx_data *data, u32 mask, unsigned long val)
375381
{
376382
switch (mask) {
377383
case INA226_SHUNT_OVER_VOLTAGE_MASK:
378384
case INA226_SHUNT_UNDER_VOLTAGE_MASK:
385+
val = clamp_val(val, 0, SHRT_MAX * data->config->shunt_div);
379386
val *= data->config->shunt_div;
380-
return clamp_val(val, SHRT_MIN, SHRT_MAX);
387+
return clamp_val(val, 0, SHRT_MAX);
381388
case INA226_BUS_OVER_VOLTAGE_MASK:
382389
case INA226_BUS_UNDER_VOLTAGE_MASK:
390+
val = clamp_val(val, 0, 200000);
383391
val = (val * 1000) << data->config->bus_voltage_shift;
384392
val = DIV_ROUND_CLOSEST(val, data->config->bus_voltage_lsb);
385-
return clamp_val(val, 0, SHRT_MAX);
393+
return clamp_val(val, 0, USHRT_MAX);
386394
case INA226_POWER_OVER_LIMIT_MASK:
395+
val = clamp_val(val, 0, UINT_MAX - data->power_lsb_uW);
387396
val = DIV_ROUND_CLOSEST(val, data->power_lsb_uW);
388397
return clamp_val(val, 0, USHRT_MAX);
389398
default:
@@ -490,19 +499,17 @@ static ssize_t ina226_alarm_show(struct device *dev,
490499
* to shunt_voltage_lsb = 1 / shunt_div multiplied by 10^9 in order
491500
* to keep the scale.
492501
*/
493-
static int ina2xx_set_shunt(struct ina2xx_data *data, long val)
502+
static int ina2xx_set_shunt(struct ina2xx_data *data, unsigned long val)
494503
{
495504
unsigned int dividend = DIV_ROUND_CLOSEST(1000000000,
496505
data->config->shunt_div);
497-
if (val <= 0 || val > dividend)
506+
if (!val || val > dividend)
498507
return -EINVAL;
499508

500-
mutex_lock(&data->config_lock);
501509
data->rshunt = val;
502510
data->current_lsb_uA = DIV_ROUND_CLOSEST(dividend, val);
503511
data->power_lsb_uW = data->config->power_lsb_factor *
504512
data->current_lsb_uA;
505-
mutex_unlock(&data->config_lock);
506513

507514
return 0;
508515
}
@@ -527,7 +534,9 @@ static ssize_t ina2xx_shunt_store(struct device *dev,
527534
if (status < 0)
528535
return status;
529536

537+
mutex_lock(&data->config_lock);
530538
status = ina2xx_set_shunt(data, val);
539+
mutex_unlock(&data->config_lock);
531540
if (status < 0)
532541
return status;
533542
return count;
@@ -545,9 +554,6 @@ static ssize_t ina226_interval_store(struct device *dev,
545554
if (status < 0)
546555
return status;
547556

548-
if (val > INT_MAX || val == 0)
549-
return -EINVAL;
550-
551557
status = regmap_update_bits(data->regmap, INA2XX_CONFIG,
552558
INA226_AVG_RD_MASK,
553559
ina226_interval_to_reg(val));
@@ -667,7 +673,9 @@ static int ina2xx_probe(struct i2c_client *client)
667673
if (device_property_read_u32(dev, "shunt-resistor", &val) < 0)
668674
val = INA2XX_RSHUNT_DEFAULT;
669675

670-
ina2xx_set_shunt(data, val);
676+
ret = ina2xx_set_shunt(data, val);
677+
if (ret < 0)
678+
return dev_err_probe(dev, ret, "Invalid shunt resistor value\n");
671679

672680
data->regmap = devm_regmap_init_i2c(client, &ina2xx_regmap_config);
673681
if (IS_ERR(data->regmap)) {

0 commit comments

Comments
 (0)