[PATCH] power: supply: bq25980: Fix uninitialized wd_reg_val and overrun

[PATCH] power: supply: bq25980: Fix uninitialized wd_reg_val and overrun

[Dan Murphy]

Fix the issue when 'i' is equal to array size then array index over
runs the array when checking for the watch dog value.

This also fixes the uninitialized wd_reg_val if the for..loop was not
successful in finding an appropriate match.

Fixes: 5069185fc18e ("power: supply: bq25980: Add support for the BQ259xx family")
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/power/supply/bq25980_charger.c | 29 +++++++++++++-------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/power/supply/bq25980_charger.c b/drivers/power/supply/bq25980_charger.c
index 3995fb7cf060..e6a91e43ae5b 100644
--- a/drivers/power/supply/bq25980_charger.c
+++ b/drivers/power/supply/bq25980_charger.c
@@ -1099,28 +1099,29 @@ static int bq25980_power_supply_init(struct bq25980_device *bq,
 static int bq25980_hw_init(struct bq25980_device *bq)
 {
 	struct power_supply_battery_info bat_info = { };
-	int wd_reg_val;
+	int wd_reg_val = BQ25980_WATCHDOG_DIS;
+	int wd_max_val = BQ25980_NUM_WD_VAL - 1;
 	int ret = 0;
 	int curr_val;
 	int volt_val;
 	int i;
 
-	if (!bq->watchdog_timer) {
-		ret = regmap_update_bits(bq->regmap, BQ25980_CHRGR_CTRL_3,
-					 BQ25980_WATCHDOG_DIS,
-					 BQ25980_WATCHDOG_DIS);
-	} else {
-		for (i = 0; i < BQ25980_NUM_WD_VAL; i++) {
-			if (bq->watchdog_timer > bq25980_watchdog_time[i] &&
-			    bq->watchdog_timer < bq25980_watchdog_time[i + 1]) {
-				wd_reg_val = i;
-				break;
+	if (bq->watchdog_timer) {
+		if (bq->watchdog_timer >= bq25980_watchdog_time[wd_max_val])
+			wd_reg_val = wd_max_val;
+		else {
+			for (i = 0; i < wd_max_val; i++) {
+				if (bq->watchdog_timer > bq25980_watchdog_time[i] &&
+				    bq->watchdog_timer < bq25980_watchdog_time[i + 1]) {
+					wd_reg_val = i;
+					break;
+				}
 			}
 		}
-
-		ret = regmap_update_bits(bq->regmap, BQ25980_CHRGR_CTRL_3,
-					BQ25980_WATCHDOG_MASK, wd_reg_val);
 	}
+
+	ret = regmap_update_bits(bq->regmap, BQ25980_CHRGR_CTRL_3,
+				 BQ25980_WATCHDOG_MASK, wd_reg_val);
 	if (ret)
 		return ret;
 
-- 
2.28.0.585.ge1cfff676549

Re: [PATCH] power: supply: bq25980: Fix uninitialized wd_reg_val and overrun

[Dan Murphy]

Sebastian

On 10/9/20 7:12 AM, Dan Murphy wrote:
> Fix the issue when 'i' is equal to array size then array index over
> runs the array when checking for the watch dog value.
>
> This also fixes the uninitialized wd_reg_val if the for..loop was not
> successful in finding an appropriate match.

Might want to pull this into next as well this is a 0-day bug fix

Dan

Re: [PATCH] power: supply: bq25980: Fix uninitialized wd_reg_val and overrun

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

Hi Dan,

On Tue, Oct 13, 2020 at 01:03:13PM -0500, Dan Murphy wrote:
> On 10/9/20 7:12 AM, Dan Murphy wrote:
> > Fix the issue when 'i' is equal to array size then array index over
> > runs the array when checking for the watch dog value.
> > 
> > This also fixes the uninitialized wd_reg_val if the for..loop was not
> > successful in finding an appropriate match.
> 
> Might want to pull this into next as well this is a 0-day bug fix

Yes, merged now. I did not take it directly, since I had to rebase
it first. Please always send power-supply patches based on the
for-next branch, which already contained a fix for the uninitialized
wd_reg_val.

(also no need to Cc DT people for this patch :))

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]