| Message ID | 4ca23609-11f4-881b-6676-83ac80dff254@dorianrudolph.com |
|---|---|
| State | Accepted |
| Commit | 093d27bb6f2d1963f927ef59c9a2d37059175426 |
| Headers | show |
| Series | power: supply: core: Fix boundary conditions in interpolation | expand |
On Sat, May 14, 2022 at 5:23 PM Dorian Rudolph <mail@dorianrudolph.com> wrote: > The functions power_supply_temp2resist_simple and power_supply_ocv2cap_simple > handle boundary conditions incorrectly. > The change was introduced in a4585ba2050f460f749bbaf2b67bd56c41e30283 > ("power: supply: core: Use library interpolation"). > There are two issues: First, the lines "high = i - 1" and "high = i" in ocv2cap > have the wrong order compared to temp2resist. As a consequence, ocv2cap > sets high=-1 if ocv>table[0].ocv, which causes an out-of-bounds read. > Second, the logic of temp2resist is also not correct. > Consider the case table[] = {{20, 100}, {10, 80}, {0, 60}}. > For temp=5, we expect a resistance of 70% by interpolation. > However, temp2resist sets high=low=2 and returns 60. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Dorian Rudolph <mail@dorianrudolph.com> My arithmetics were not with me that day. I also copypasted the error :( Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Fixes: a4585ba2050f ("power: supply: core: Use library interpolation") Cc: stable@vger.kernel.org Yours, Linus Walleij
Hi, On Sun, May 15, 2022 at 12:09:56AM +0200, Linus Walleij wrote: > On Sat, May 14, 2022 at 5:23 PM Dorian Rudolph <mail@dorianrudolph.com> wrote: > > > The functions power_supply_temp2resist_simple and power_supply_ocv2cap_simple > > handle boundary conditions incorrectly. > > The change was introduced in a4585ba2050f460f749bbaf2b67bd56c41e30283 > > ("power: supply: core: Use library interpolation"). > > There are two issues: First, the lines "high = i - 1" and "high = i" in ocv2cap > > have the wrong order compared to temp2resist. As a consequence, ocv2cap > > sets high=-1 if ocv>table[0].ocv, which causes an out-of-bounds read. > > Second, the logic of temp2resist is also not correct. > > Consider the case table[] = {{20, 100}, {10, 80}, {0, 60}}. > > For temp=5, we expect a resistance of 70% by interpolation. > > However, temp2resist sets high=low=2 and returns 60. > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Dorian Rudolph <mail@dorianrudolph.com> > > My arithmetics were not with me that day. I also copypasted the error :( > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Fixes: a4585ba2050f ("power: supply: core: Use library interpolation") > Cc: stable@vger.kernel.org Thanks, queued to power-supply's fixes branch. -- Sebastian
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c index ec838c9bcc0a..3828ba9d0eab 100644 --- a/drivers/power/supply/power_supply_core.c +++ b/drivers/power/supply/power_supply_core.c @@ -801,17 +801,17 @@ int power_supply_temp2resist_simple(struct power_supply_resistance_temp_table *t { int i, high, low; - /* Break loop at table_len - 1 because that is the highest index */ - for (i = 0; i < table_len - 1; i++) + for (i = 0; i < table_len; i++) if (temp > table[i].temp) break; /* The library function will deal with high == low */ - if ((i == 0) || (i == (table_len - 1))) - high = i; + if (i == 0) + high = low = i; + else if (i == table_len) + high = low = i - 1; else - high = i - 1; - low = i; + high = (low = i) - 1; return fixp_linear_interpolate(table[low].temp, table[low].resistance, @@ -838,17 +838,17 @@ int power_supply_ocv2cap_simple(struct power_supply_battery_ocv_table *table, { int i, high, low; - /* Break loop at table_len - 1 because that is the highest index */ - for (i = 0; i < table_len - 1; i++) + for (i = 0; i < table_len; i++) if (ocv > table[i].ocv) break; /* The library function will deal with high == low */ - if ((i == 0) || (i == (table_len - 1))) - high = i - 1; + if (i == 0) + high = low = i; + else if (i == table_len) + high = low = i - 1; else - high = i; /* i.e. i == 0 */ - low = i; + high = (low = i) - 1; return fixp_linear_interpolate(table[low].ocv, table[low].capacity,
The functions power_supply_temp2resist_simple and power_supply_ocv2cap_simple handle boundary conditions incorrectly. The change was introduced in a4585ba2050f460f749bbaf2b67bd56c41e30283 ("power: supply: core: Use library interpolation"). There are two issues: First, the lines "high = i - 1" and "high = i" in ocv2cap have the wrong order compared to temp2resist. As a consequence, ocv2cap sets high=-1 if ocv>table[0].ocv, which causes an out-of-bounds read. Second, the logic of temp2resist is also not correct. Consider the case table[] = {{20, 100}, {10, 80}, {0, 60}}. For temp=5, we expect a resistance of 70% by interpolation. However, temp2resist sets high=low=2 and returns 60. Cc: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Dorian Rudolph <mail@dorianrudolph.com> --- drivers/power/supply/power_supply_core.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)