| Message ID | 20240321145609.8159-2-bethany.jamison@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | CVE-2020-36776 | expand |
On Thu, Mar 21, 2024 at 09:56:09AM -0500, Bethany Jamison wrote: > From: brian-sy yang <brian-sy.yang@mediatek.com> > > Slab OOB issue is scanned by KASAN in cpu_power_to_freq(). > If power is limited below the power of OPP0 in EM table, > it will cause slab out-of-bound issue with negative array > index. > > Return the lowest frequency if limited power cannot found > a suitable OPP in EM table to fix this issue. > > Backtrace: > [<ffffffd02d2a37f0>] die+0x104/0x5ac > [<ffffffd02d2a5630>] bug_handler+0x64/0xd0 > [<ffffffd02d288ce4>] brk_handler+0x160/0x258 > [<ffffffd02d281e5c>] do_debug_exception+0x248/0x3f0 > [<ffffffd02d284488>] el1_dbg+0x14/0xbc > [<ffffffd02d75d1d4>] __kasan_report+0x1dc/0x1e0 > [<ffffffd02d75c2e0>] kasan_report+0x10/0x20 > [<ffffffd02d75def8>] __asan_report_load8_noabort+0x18/0x28 > [<ffffffd02e6fce5c>] cpufreq_power2state+0x180/0x43c > [<ffffffd02e6ead80>] power_actor_set_power+0x114/0x1d4 > [<ffffffd02e6fac24>] allocate_power+0xaec/0xde0 > [<ffffffd02e6f9f80>] power_allocator_throttle+0x3ec/0x5a4 > [<ffffffd02e6ea888>] handle_thermal_trip+0x160/0x294 > [<ffffffd02e6edd08>] thermal_zone_device_check+0xe4/0x154 > [<ffffffd02d351cb4>] process_one_work+0x5e4/0xe28 > [<ffffffd02d352f44>] worker_thread+0xa4c/0xfac > [<ffffffd02d360124>] kthread+0x33c/0x358 > [<ffffffd02d289940>] ret_from_fork+0xc/0x18 > > Fixes: 371a3bc79c11b ("thermal/drivers/cpufreq_cooling: Fix wrong frequency converted from power") > Signed-off-by: brian-sy yang <brian-sy.yang@mediatek.com> > Signed-off-by: Michael Kao <michael.kao@mediatek.com> > Reviewed-by: Lukasz Luba <lukasz.luba@arm.com> > Cc: stable@vger.kernel.org #v5.7 > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > Link: https://lore.kernel.org/r/20201229050831.19493-1-michael.kao@mediatek.com > (backported from commit 34ab17cc6c2c1ac93d7e5d53bb972df9a968f085) > [bjamison: context conflict - fix commit changes the range of a for-loop from > [0, max_level] to (0, max_level], Focal's for-loop is set-up differently > than upstream, I changed the range from [0, max_level) to match the fix range] > CVE-2020-36776 > Signed-off-by: Bethany Jamison <bethany.jamison@canonical.com> > --- > drivers/thermal/cpu_cooling.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c > index 9d24bc05df0da..ba3df71bb877f 100644 > --- a/drivers/thermal/cpu_cooling.c > +++ b/drivers/thermal/cpu_cooling.c > @@ -210,7 +210,7 @@ static u32 cpu_power_to_freq(struct cpufreq_cooling_device *cpufreq_cdev, > int i; > struct freq_table *freq_table = cpufreq_cdev->freq_table; > > - for (i = 0; i < cpufreq_cdev->max_level; i++) > + for (i = 1; i <= cpufreq_cdev->max_level; i++) In the scenario that this patch is supposed to fix, where power < freq_table[0].power, this backport will access freq_table[i].frequency, where i == cpufreq_cdev->max_level + 1, resulting in an invalid memory access. Furthermore, by starting the loop at i == 1, the function will never return the frequency at index 0 of the table even if the corresponding power is below the limit. I don't think bionic/focal are affected by the described out-of-bounds issue and this backport is not required. cpu_power_to_freq() will return the highest frequency instead of the lowest in case power is limited below what is specified in the table for bionic/focal as opposed to later kernel series, but that is a separate issue. Applying this fix only makes sense if were to also backport the prerequisite changes that have been introduced in later kernel series, as far as I can tell. > if (power >= freq_table[i].power) > break; > > -- > 2.34.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 25.03.24 15:41, Manuel Diewald wrote: > On Thu, Mar 21, 2024 at 09:56:09AM -0500, Bethany Jamison wrote: >> From: brian-sy yang <brian-sy.yang@mediatek.com> >> >> Slab OOB issue is scanned by KASAN in cpu_power_to_freq(). >> If power is limited below the power of OPP0 in EM table, >> it will cause slab out-of-bound issue with negative array >> index. >> >> Return the lowest frequency if limited power cannot found >> a suitable OPP in EM table to fix this issue. >> >> Backtrace: >> [<ffffffd02d2a37f0>] die+0x104/0x5ac >> [<ffffffd02d2a5630>] bug_handler+0x64/0xd0 >> [<ffffffd02d288ce4>] brk_handler+0x160/0x258 >> [<ffffffd02d281e5c>] do_debug_exception+0x248/0x3f0 >> [<ffffffd02d284488>] el1_dbg+0x14/0xbc >> [<ffffffd02d75d1d4>] __kasan_report+0x1dc/0x1e0 >> [<ffffffd02d75c2e0>] kasan_report+0x10/0x20 >> [<ffffffd02d75def8>] __asan_report_load8_noabort+0x18/0x28 >> [<ffffffd02e6fce5c>] cpufreq_power2state+0x180/0x43c >> [<ffffffd02e6ead80>] power_actor_set_power+0x114/0x1d4 >> [<ffffffd02e6fac24>] allocate_power+0xaec/0xde0 >> [<ffffffd02e6f9f80>] power_allocator_throttle+0x3ec/0x5a4 >> [<ffffffd02e6ea888>] handle_thermal_trip+0x160/0x294 >> [<ffffffd02e6edd08>] thermal_zone_device_check+0xe4/0x154 >> [<ffffffd02d351cb4>] process_one_work+0x5e4/0xe28 >> [<ffffffd02d352f44>] worker_thread+0xa4c/0xfac >> [<ffffffd02d360124>] kthread+0x33c/0x358 >> [<ffffffd02d289940>] ret_from_fork+0xc/0x18 >> >> Fixes: 371a3bc79c11b ("thermal/drivers/cpufreq_cooling: Fix wrong frequency converted from power") >> Signed-off-by: brian-sy yang <brian-sy.yang@mediatek.com> >> Signed-off-by: Michael Kao <michael.kao@mediatek.com> >> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com> >> Cc: stable@vger.kernel.org #v5.7 >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> Link: https://lore.kernel.org/r/20201229050831.19493-1-michael.kao@mediatek.com >> (backported from commit 34ab17cc6c2c1ac93d7e5d53bb972df9a968f085) >> [bjamison: context conflict - fix commit changes the range of a for-loop from >> [0, max_level] to (0, max_level], Focal's for-loop is set-up differently >> than upstream, I changed the range from [0, max_level) to match the fix range] >> CVE-2020-36776 >> Signed-off-by: Bethany Jamison <bethany.jamison@canonical.com> >> --- >> drivers/thermal/cpu_cooling.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c >> index 9d24bc05df0da..ba3df71bb877f 100644 >> --- a/drivers/thermal/cpu_cooling.c >> +++ b/drivers/thermal/cpu_cooling.c >> @@ -210,7 +210,7 @@ static u32 cpu_power_to_freq(struct cpufreq_cooling_device *cpufreq_cdev, >> int i; >> struct freq_table *freq_table = cpufreq_cdev->freq_table; >> >> - for (i = 0; i < cpufreq_cdev->max_level; i++) >> + for (i = 1; i <= cpufreq_cdev->max_level; i++) > > In the scenario that this patch is supposed to fix, where power < > freq_table[0].power, this backport will access freq_table[i].frequency, > where i == cpufreq_cdev->max_level + 1, resulting in an invalid memory > access. Furthermore, by starting the loop at i == 1, the function will > never return the frequency at index 0 of the table even if the > corresponding power is below the limit. > > I don't think bionic/focal are affected by the described out-of-bounds > issue and this backport is not required. > > cpu_power_to_freq() will return the highest frequency instead of the > lowest in case power is limited below what is specified in the table for > bionic/focal as opposed to later kernel series, but that is a separate > issue. Applying this fix only makes sense if were to also backport the > prerequisite changes that have been introduced in later kernel series, > as far as I can tell. > >> if (power >= freq_table[i].power) >> break; >> >> -- >> 2.34.1 >> >> >> -- >> kernel-team mailing list >> kernel-team@lists.ubuntu.com >> https://lists.ubuntu.com/mailman/listinfo/kernel-team > Ok, I have dropped this patch from Focal for now.
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 9d24bc05df0da..ba3df71bb877f 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -210,7 +210,7 @@ static u32 cpu_power_to_freq(struct cpufreq_cooling_device *cpufreq_cdev, int i; struct freq_table *freq_table = cpufreq_cdev->freq_table; - for (i = 0; i < cpufreq_cdev->max_level; i++) + for (i = 1; i <= cpufreq_cdev->max_level; i++) if (power >= freq_table[i].power) break;