2021-05-03 05:10:19

by Kees Cook

Subject: [PATCH 0/2] drm/radeon: Fix off-by-one power_state index heap overwrite

Hi,

This is an attempt at fixing a bug[1] uncovered by the relocation of
the slab freelist pointer offset, as well as some related clean-ups.

I don't have hardware to do runtime testing, but it builds. ;)

-Kees

[1] https://bugzilla.kernel.org/show_bug.cgi?id=211537

Kees Cook (2):
drm/radeon: Fix off-by-one power_state index heap overwrite
drm/radeon: Avoid power table parsing memory leaks

drivers/gpu/drm/radeon/radeon_atombios.c | 26 ++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)

--
2.25.1

2021-05-03 05:12:17

by Kees Cook

Subject: [PATCH 2/2] drm/radeon: Avoid power table parsing memory leaks

Avoid leaving a hanging pre-allocated clock_info if last mode is
invalid, and avoid heap corruption if no valid modes are found.

Fixes: 6991b8f2a319 ("drm/radeon/kms: fix segfault in pm rework")
Signed-off-by: Kees Cook <[email protected]>
---
drivers/gpu/drm/radeon/radeon_atombios.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c b/drivers/gpu/drm/radeon/radeon_atombios.c
index f9f4efa1738c..28c4413f4dc8 100644
--- a/drivers/gpu/drm/radeon/radeon_atombios.c
+++ b/drivers/gpu/drm/radeon/radeon_atombios.c
@@ -2120,11 +2120,14 @@ static int radeon_atombios_parse_power_table_1_3(struct radeon_device *rdev)
return state_index;
/* last mode is usually default, array is low to high */
for (i = 0; i < num_modes; i++) {
- rdev->pm.power_state[state_index].clock_info =
- kcalloc(1, sizeof(struct radeon_pm_clock_info),
- GFP_KERNEL);
+ /* avoid memory leaks from invalid modes or unknown frev. */
+ if (!rdev->pm.power_state[state_index].clock_info) {
+ rdev->pm.power_state[state_index].clock_info =
+ kzalloc(sizeof(struct radeon_pm_clock_info),
+ GFP_KERNEL);
+ }
if (!rdev->pm.power_state[state_index].clock_info)
- return state_index;
+ goto out;
rdev->pm.power_state[state_index].num_clock_modes = 1;
rdev->pm.power_state[state_index].clock_info[0].voltage.type = VOLTAGE_NONE;
switch (frev) {
@@ -2243,8 +2246,15 @@ static int radeon_atombios_parse_power_table_1_3(struct radeon_device *rdev)
break;
}
}
+out:
+ /* free any unused clock_info allocation. */
+ if (state_index && state_index < num_modes) {
+ kfree(rdev->pm.power_state[state_index].clock_info);
+ rdev->pm.power_state[state_index].clock_info = NULL;
+ }
+
/* last mode is usually default */
- if (rdev->pm.default_power_state_index == -1) {
+ if (state_index && rdev->pm.default_power_state_index == -1) {
rdev->pm.power_state[state_index - 1].type =
POWER_STATE_TYPE_DEFAULT;
rdev->pm.default_power_state_index = state_index - 1;
--
2.25.1

2021-05-04 19:13:38

by Alex Deucher

Subject: Re: [PATCH 0/2] drm/radeon: Fix off-by-one power_state index heap overwrite

On Mon, May 3, 2021 at 1:06 AM Kees Cook <[email protected]> wrote:
>
> Hi,
>
> This is an attempt at fixing a bug[1] uncovered by the relocation of
> the slab freelist pointer offset, as well as some related clean-ups.
>
> I don't have hardware to do runtime testing, but it builds. ;)
>
> -Kees
>
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=211537
>
> Kees Cook (2):
> drm/radeon: Fix off-by-one power_state index heap overwrite
> drm/radeon: Avoid power table parsing memory leaks

Applied. Thanks!

Alex

>
> drivers/gpu/drm/radeon/radeon_atombios.c | 26 ++++++++++++++++--------
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel