| Message ID | 20241128-topic-opp-fix-assert-index-check-v1-2-cb8bd4c0370e@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | OPP: fix buffer overflow in indexed freq and bandwidth reads | expand |
On 29/11/2024 09:41, Viresh Kumar wrote: > On 28-11-24, 11:07, Neil Armstrong wrote: >> If a driver calls dev_pm_opp_find_bw_ceil/floor() the retrieve bandwidth >> from the OPP table but the bandwidth table was not created because the >> interconnect properties were missing in the OPP consumer node, the >> kernel will crash with: >> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004 >> ... >> pc : _read_bw+0x8/0x10 >> lr : _opp_table_find_key+0x9c/0x174 >> ... >> Call trace: >> _read_bw+0x8/0x10 (P) >> _opp_table_find_key+0x9c/0x174 (L) >> _find_key+0x98/0x168 >> dev_pm_opp_find_bw_ceil+0x50/0x88 >> ... >> >> In order to fix the crash, create an assert function to check >> if the bandwidth table were created before trying to get a >> bandwidth with _read_bw(). >> >> Fixes: add1dc094a74 ("OPP: Use generic key finding helpers for bandwidth key") >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> drivers/opp/core.c | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/opp/core.c b/drivers/opp/core.c >> index 8692e8ce05b7c31a725ea3a7928f238c7a1d6f51..178780e294dad49c22d866930efb7b8b13ae2d61 100644 >> --- a/drivers/opp/core.c >> +++ b/drivers/opp/core.c >> @@ -114,6 +114,14 @@ static bool assert_clk_index(struct opp_table *opp_table, int index) >> return opp_table->clk_count > index; >> } >> >> +/* >> + * Returns true if bandwidth table is large enough to contain the bandwidth index. >> + */ >> +static bool assert_bandwidth_index(struct opp_table *opp_table, int index) >> +{ >> + return opp_table->path_count > index; >> +} >> + >> /** >> * dev_pm_opp_get_bw() - Gets the bandwidth corresponding to an opp >> * @opp: opp for which bandwidth has to be returned for >> @@ -913,7 +921,8 @@ struct dev_pm_opp *dev_pm_opp_find_bw_ceil(struct device *dev, unsigned int *bw, >> unsigned long temp = *bw; >> struct dev_pm_opp *opp; >> >> - opp = _find_key_ceil(dev, &temp, index, true, _read_bw, NULL); >> + opp = _find_key_ceil(dev, &temp, index, true, _read_bw, >> + assert_bandwidth_index); >> *bw = temp; >> return opp; >> } >> @@ -944,7 +953,8 @@ struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev, >> unsigned long temp = *bw; >> struct dev_pm_opp *opp; >> >> - opp = _find_key_floor(dev, &temp, index, true, _read_bw, NULL); >> + opp = _find_key_floor(dev, &temp, index, true, _read_bw, >> + assert_bandwidth_index); >> *bw = temp; >> return opp; >> } > > Applied. Thanks. > Wait, this needs the first patch to work, otherwise index is not passed to assert Neil
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 8692e8ce05b7c31a725ea3a7928f238c7a1d6f51..178780e294dad49c22d866930efb7b8b13ae2d61 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -114,6 +114,14 @@ static bool assert_clk_index(struct opp_table *opp_table, int index) return opp_table->clk_count > index; } +/* + * Returns true if bandwidth table is large enough to contain the bandwidth index. + */ +static bool assert_bandwidth_index(struct opp_table *opp_table, int index) +{ + return opp_table->path_count > index; +} + /** * dev_pm_opp_get_bw() - Gets the bandwidth corresponding to an opp * @opp: opp for which bandwidth has to be returned for @@ -913,7 +921,8 @@ struct dev_pm_opp *dev_pm_opp_find_bw_ceil(struct device *dev, unsigned int *bw, unsigned long temp = *bw; struct dev_pm_opp *opp; - opp = _find_key_ceil(dev, &temp, index, true, _read_bw, NULL); + opp = _find_key_ceil(dev, &temp, index, true, _read_bw, + assert_bandwidth_index); *bw = temp; return opp; } @@ -944,7 +953,8 @@ struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev, unsigned long temp = *bw; struct dev_pm_opp *opp; - opp = _find_key_floor(dev, &temp, index, true, _read_bw, NULL); + opp = _find_key_floor(dev, &temp, index, true, _read_bw, + assert_bandwidth_index); *bw = temp; return opp; }
If a driver calls dev_pm_opp_find_bw_ceil/floor() the retrieve bandwidth from the OPP table but the bandwidth table was not created because the interconnect properties were missing in the OPP consumer node, the kernel will crash with: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004 ... pc : _read_bw+0x8/0x10 lr : _opp_table_find_key+0x9c/0x174 ... Call trace: _read_bw+0x8/0x10 (P) _opp_table_find_key+0x9c/0x174 (L) _find_key+0x98/0x168 dev_pm_opp_find_bw_ceil+0x50/0x88 ... In order to fix the crash, create an assert function to check if the bandwidth table were created before trying to get a bandwidth with _read_bw(). Fixes: add1dc094a74 ("OPP: Use generic key finding helpers for bandwidth key") Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- drivers/opp/core.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)