Re: [PATCH] drm/amdgpu: replace 1-element arrays with flexible arrays

From: Alex Deucher
Date: Wed Jul 12 2023 - 10:12:25 EST

On Wed, Jul 12, 2023 at 8:04 AM Ricardo Cañuelo
<ricardo.canuelo@xxxxxxxxxxxxx> wrote:
>
> UBSAN complains about out-of-bounds array indexes on all 1-element
> arrays defined on this driver:
>
> UBSAN: array-index-out-of-bounds in /home/rcn/work/repos/kernelci/kernelci-core/linux_kernel_mainline/drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/processpptables.c:1249:61
>
> Substitute them with proper flexible arrays.

+ Gustavo, Paulo

I haven't kept up with the flexible arrays stuff. Is this equivalent
to a zero sized array? We've been bitten by these kind of changes in
the past. These structures define the layout of data in a rom image
on the board. If the struct size changes, that could lead to errors
in the code that deals with these structures.

Alex

>
> Tested on an Acer R721T (grunt) Chromebook.
>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@xxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/amd/include/pptable.h | 36 +++++++++++++++------------
> 1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/include/pptable.h b/drivers/gpu/drm/amd/include/pptable.h
> index 0b6a057e0a4c..a65e2807dc06 100644
> --- a/drivers/gpu/drm/amd/include/pptable.h
> +++ b/drivers/gpu/drm/amd/include/pptable.h
> @@ -473,14 +473,14 @@ typedef struct _ATOM_PPLIB_STATE_V2
> /**
> * Driver will read the first ucNumDPMLevels in this array
> */
> - UCHAR clockInfoIndex[1];
> + __DECLARE_FLEX_ARRAY(UCHAR, clockInfoIndex);
> } ATOM_PPLIB_STATE_V2;
>
> typedef struct _StateArray{
> //how many states we have
> UCHAR ucNumEntries;
>
> - ATOM_PPLIB_STATE_V2 states[1];
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_STATE_V2, states);
> }StateArray;
>
>
> @@ -491,7 +491,7 @@ typedef struct _ClockInfoArray{
> //sizeof(ATOM_PPLIB_CLOCK_INFO)
> UCHAR ucEntrySize;
>
> - UCHAR clockInfo[1];
> + __DECLARE_FLEX_ARRAY(UCHAR, clockInfo);
> }ClockInfoArray;
>
> typedef struct _NonClockInfoArray{
> @@ -501,7 +501,7 @@ typedef struct _NonClockInfoArray{
> //sizeof(ATOM_PPLIB_NONCLOCK_INFO)
> UCHAR ucEntrySize;
>
> - ATOM_PPLIB_NONCLOCK_INFO nonClockInfo[1];
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_NONCLOCK_INFO, nonClockInfo);
> }NonClockInfoArray;
>
> typedef struct _ATOM_PPLIB_Clock_Voltage_Dependency_Record
> @@ -514,7 +514,8 @@ typedef struct _ATOM_PPLIB_Clock_Voltage_Dependency_Record
> typedef struct _ATOM_PPLIB_Clock_Voltage_Dependency_Table
> {
> UCHAR ucNumEntries; // Number of entries.
> - ATOM_PPLIB_Clock_Voltage_Dependency_Record entries[1]; // Dynamically allocate entries.
> + /* Dynamically allocate entries. */
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_Clock_Voltage_Dependency_Record, entries);
> }ATOM_PPLIB_Clock_Voltage_Dependency_Table;
>
> typedef struct _ATOM_PPLIB_Clock_Voltage_Limit_Record
> @@ -530,7 +531,8 @@ typedef struct _ATOM_PPLIB_Clock_Voltage_Limit_Record
> typedef struct _ATOM_PPLIB_Clock_Voltage_Limit_Table
> {
> UCHAR ucNumEntries; // Number of entries.
> - ATOM_PPLIB_Clock_Voltage_Limit_Record entries[1]; // Dynamically allocate entries.
> + /* Dynamically allocate entries. */
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_Clock_Voltage_Limit_Record, entries);
> }ATOM_PPLIB_Clock_Voltage_Limit_Table;
>
> union _ATOM_PPLIB_CAC_Leakage_Record
> @@ -554,7 +556,8 @@ typedef union _ATOM_PPLIB_CAC_Leakage_Record ATOM_PPLIB_CAC_Leakage_Record;
> typedef struct _ATOM_PPLIB_CAC_Leakage_Table
> {
> UCHAR ucNumEntries; // Number of entries.
> - ATOM_PPLIB_CAC_Leakage_Record entries[1]; // Dynamically allocate entries.
> + /* Dynamically allocate entries. */
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_CAC_Leakage_Record, entries);
> }ATOM_PPLIB_CAC_Leakage_Table;
>
> typedef struct _ATOM_PPLIB_PhaseSheddingLimits_Record
> @@ -569,7 +572,8 @@ typedef struct _ATOM_PPLIB_PhaseSheddingLimits_Record
> typedef struct _ATOM_PPLIB_PhaseSheddingLimits_Table
> {
> UCHAR ucNumEntries; // Number of entries.
> - ATOM_PPLIB_PhaseSheddingLimits_Record entries[1]; // Dynamically allocate entries.
> + /* Dynamically allocate entries. */
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_PhaseSheddingLimits_Record, entries);
> }ATOM_PPLIB_PhaseSheddingLimits_Table;
>
> typedef struct _VCEClockInfo{
> @@ -581,7 +585,7 @@ typedef struct _VCEClockInfo{
>
> typedef struct _VCEClockInfoArray{
> UCHAR ucNumEntries;
> - VCEClockInfo entries[1];
> + __DECLARE_FLEX_ARRAY(VCEClockInfo, entries);
> }VCEClockInfoArray;
>
> typedef struct _ATOM_PPLIB_VCE_Clock_Voltage_Limit_Record
> @@ -593,7 +597,7 @@ typedef struct _ATOM_PPLIB_VCE_Clock_Voltage_Limit_Record
> typedef struct _ATOM_PPLIB_VCE_Clock_Voltage_Limit_Table
> {
> UCHAR numEntries;
> - ATOM_PPLIB_VCE_Clock_Voltage_Limit_Record entries[1];
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_VCE_Clock_Voltage_Limit_Record, entries);
> }ATOM_PPLIB_VCE_Clock_Voltage_Limit_Table;
>
> typedef struct _ATOM_PPLIB_VCE_State_Record
> @@ -605,7 +609,7 @@ typedef struct _ATOM_PPLIB_VCE_State_Record
> typedef struct _ATOM_PPLIB_VCE_State_Table
> {
> UCHAR numEntries;
> - ATOM_PPLIB_VCE_State_Record entries[1];
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_VCE_State_Record, entries);
> }ATOM_PPLIB_VCE_State_Table;
>
>
> @@ -627,7 +631,7 @@ typedef struct _UVDClockInfo{
>
> typedef struct _UVDClockInfoArray{
> UCHAR ucNumEntries;
> - UVDClockInfo entries[1];
> + __DECLARE_FLEX_ARRAY(UVDClockInfo, entries);
> }UVDClockInfoArray;
>
> typedef struct _ATOM_PPLIB_UVD_Clock_Voltage_Limit_Record
> @@ -639,7 +643,7 @@ typedef struct _ATOM_PPLIB_UVD_Clock_Voltage_Limit_Record
> typedef struct _ATOM_PPLIB_UVD_Clock_Voltage_Limit_Table
> {
> UCHAR numEntries;
> - ATOM_PPLIB_UVD_Clock_Voltage_Limit_Record entries[1];
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_UVD_Clock_Voltage_Limit_Record, entries);
> }ATOM_PPLIB_UVD_Clock_Voltage_Limit_Table;
>
> typedef struct _ATOM_PPLIB_UVD_Table
> @@ -658,7 +662,7 @@ typedef struct _ATOM_PPLIB_SAMClk_Voltage_Limit_Record
>
> typedef struct _ATOM_PPLIB_SAMClk_Voltage_Limit_Table{
> UCHAR numEntries;
> - ATOM_PPLIB_SAMClk_Voltage_Limit_Record entries[1];
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_SAMClk_Voltage_Limit_Record, entries);
> }ATOM_PPLIB_SAMClk_Voltage_Limit_Table;
>
> typedef struct _ATOM_PPLIB_SAMU_Table
> @@ -676,7 +680,7 @@ typedef struct _ATOM_PPLIB_ACPClk_Voltage_Limit_Record
>
> typedef struct _ATOM_PPLIB_ACPClk_Voltage_Limit_Table{
> UCHAR numEntries;
> - ATOM_PPLIB_ACPClk_Voltage_Limit_Record entries[1];
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_ACPClk_Voltage_Limit_Record, entries);
> }ATOM_PPLIB_ACPClk_Voltage_Limit_Table;
>
> typedef struct _ATOM_PPLIB_ACP_Table
> @@ -745,7 +749,7 @@ typedef struct ATOM_PPLIB_VQ_Budgeting_Record{
> typedef struct ATOM_PPLIB_VQ_Budgeting_Table {
> UCHAR revid;
> UCHAR numEntries;
> - ATOM_PPLIB_VQ_Budgeting_Record entries[1];
> + __DECLARE_FLEX_ARRAY(ATOM_PPLIB_VQ_Budgeting_Record, entries);
> } ATOM_PPLIB_VQ_Budgeting_Table;
>
> #pragma pack()
> --
> 2.25.1
>