Commit 564ac172 authored by Gustavo A. R. Silva's avatar Gustavo A. R. Silva Committed by Alex Deucher

drm/amd/pm: Fix out-of-bounds bug

Create new structure SISLANDS_SMC_SWSTATE_SINGLE, as initialState.levels
and ACPIState.levels are never actually used as flexible arrays. Those
arrays can be used as simple objects of type
SISLANDS_SMC_HW_PERFORMANCE_LEVEL, instead.

Currently, the code fails because flexible array _levels_ in
struct SISLANDS_SMC_SWSTATE doesn't allow for code that accesses
the first element of initialState.levels and ACPIState.levels
arrays:

drivers/gpu/drm/amd/pm/powerplay/si_dpm.c:
4820: table->initialState.levels[0].mclk.vDLL_CNTL =
4821:         cpu_to_be32(si_pi->clock_registers.dll_cntl);
...
5021: table->ACPIState.levels[0].mclk.vDLL_CNTL =
5022:         cpu_to_be32(dll_cntl);

because such element cannot be accessed without previously allocating
enough dynamic memory for it to exist (which never actually happens).
So, there is an out-of-bounds bug in this case.

That's why struct SISLANDS_SMC_SWSTATE should only be used as type
for object driverState and new struct SISLANDS_SMC_SWSTATE_SINGLE is
created as type for objects initialState, ACPIState and ULVState.

Also, with the change from one-element array to flexible-array member
in commit 0e1aa13c ("drm/amd/pm: Replace one-element array with
flexible-array in struct SISLANDS_SMC_SWSTATE"), the size of
dpmLevels in struct SISLANDS_SMC_STATETABLE should be fixed to be
SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE instead of
SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1.

Fixes: 0e1aa13c ("drm/amd/pm: Replace one-element array with flexible-array in struct SISLANDS_SMC_SWSTATE")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarGustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent cc1a54d8
This diff is collapsed.
......@@ -191,6 +191,14 @@ struct SISLANDS_SMC_SWSTATE
typedef struct SISLANDS_SMC_SWSTATE SISLANDS_SMC_SWSTATE;
struct SISLANDS_SMC_SWSTATE_SINGLE {
uint8_t flags;
uint8_t levelCount;
uint8_t padding2;
uint8_t padding3;
SISLANDS_SMC_HW_PERFORMANCE_LEVEL level;
};
#define SISLANDS_SMC_VOLTAGEMASK_VDDC 0
#define SISLANDS_SMC_VOLTAGEMASK_MVDD 1
#define SISLANDS_SMC_VOLTAGEMASK_VDDCI 2
......@@ -216,11 +224,11 @@ struct SISLANDS_SMC_STATETABLE
SISLANDS_SMC_VOLTAGEMASKTABLE voltageMaskTable;
SISLANDS_SMC_VOLTAGEMASKTABLE phaseMaskTable;
PP_SIslands_DPM2Parameters dpm2Params;
SISLANDS_SMC_SWSTATE initialState;
SISLANDS_SMC_SWSTATE ACPIState;
SISLANDS_SMC_SWSTATE ULVState;
struct SISLANDS_SMC_SWSTATE_SINGLE initialState;
struct SISLANDS_SMC_SWSTATE_SINGLE ACPIState;
struct SISLANDS_SMC_SWSTATE_SINGLE ULVState;
SISLANDS_SMC_SWSTATE driverState;
SISLANDS_SMC_HW_PERFORMANCE_LEVEL dpmLevels[SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1];
SISLANDS_SMC_HW_PERFORMANCE_LEVEL dpmLevels[SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE];
};
typedef struct SISLANDS_SMC_STATETABLE SISLANDS_SMC_STATETABLE;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment