[PATCH v2] drm/amdgpu/pptable: Fix UBSAN array-index-out-of-bounds

[Alex Deucher]

Applied.  Thanks!

On Fri, May 31, 2024 at 12:31 PM Tasos Sahanidis <tasos at tasossah.com> wrote:
>
> Flexible arrays used [1] instead of []. Replace the former with the latter
> to resolve multiple UBSAN warnings observed on boot with a BONAIRE card.
>
> In addition, use the __counted_by attribute where possible to hint the
> length of the arrays to the compiler and any sanitizers.
>
> Signed-off-by: Tasos Sahanidis <tasos at tasossah.com>
> ---
> V1 -> V2: Added the __counted_by attribute where possible and reworded
>           the commit message.
>
>  drivers/gpu/drm/amd/include/pptable.h | 91 ++++++++++++++-------------
>  1 file changed, 49 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/include/pptable.h b/drivers/gpu/drm/amd/include/pptable.h
> index 2e8e6c9875f6..f83ace2d7ec3 100644
> --- a/drivers/gpu/drm/amd/include/pptable.h
> +++ b/drivers/gpu/drm/amd/include/pptable.h
> @@ -477,31 +477,30 @@ typedef struct _ATOM_PPLIB_STATE_V2
>  } ATOM_PPLIB_STATE_V2;
>
>  typedef struct _StateArray{
> -    //how many states we have
> -    UCHAR ucNumEntries;
> -
> -    ATOM_PPLIB_STATE_V2 states[1];
> +       //how many states we have
> +       UCHAR ucNumEntries;
> +
> +       ATOM_PPLIB_STATE_V2 states[] /* __counted_by(ucNumEntries) */;
>  }StateArray;
>
>
>  typedef struct _ClockInfoArray{
> -    //how many clock levels we have
> -    UCHAR ucNumEntries;
> -
> -    //sizeof(ATOM_PPLIB_CLOCK_INFO)
> -    UCHAR ucEntrySize;
> -
> -    UCHAR clockInfo[];
> +       //how many clock levels we have
> +       UCHAR ucNumEntries;
> +
> +       //sizeof(ATOM_PPLIB_CLOCK_INFO)
> +       UCHAR ucEntrySize;
> +
> +       UCHAR clockInfo[];
>  }ClockInfoArray;
>
>  typedef struct _NonClockInfoArray{
> +       //how many non-clock levels we have. normally should be same as number of states
> +       UCHAR ucNumEntries;
> +       //sizeof(ATOM_PPLIB_NONCLOCK_INFO)
> +       UCHAR ucEntrySize;
>
> -    //how many non-clock levels we have. normally should be same as number of states
> -    UCHAR ucNumEntries;
> -    //sizeof(ATOM_PPLIB_NONCLOCK_INFO)
> -    UCHAR ucEntrySize;
> -
> -    ATOM_PPLIB_NONCLOCK_INFO nonClockInfo[];
> +       ATOM_PPLIB_NONCLOCK_INFO nonClockInfo[] __counted_by(ucNumEntries);
>  }NonClockInfoArray;
>
>  typedef struct _ATOM_PPLIB_Clock_Voltage_Dependency_Record
> @@ -513,8 +512,10 @@ 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.
> +       // Number of entries.
> +       UCHAR ucNumEntries;
> +       // Dynamically allocate entries.
> +       ATOM_PPLIB_Clock_Voltage_Dependency_Record entries[] __counted_by(ucNumEntries);
>  }ATOM_PPLIB_Clock_Voltage_Dependency_Table;
>
>  typedef struct _ATOM_PPLIB_Clock_Voltage_Limit_Record
> @@ -529,8 +530,10 @@ 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.
> +       // Number of entries.
> +       UCHAR ucNumEntries;
> +       // Dynamically allocate entries.
> +       ATOM_PPLIB_Clock_Voltage_Limit_Record entries[] __counted_by(ucNumEntries);
>  }ATOM_PPLIB_Clock_Voltage_Limit_Table;
>
>  union _ATOM_PPLIB_CAC_Leakage_Record
> @@ -553,8 +556,10 @@ 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.
> +       // Number of entries.
> +       UCHAR ucNumEntries;
> +       // Dynamically allocate entries.
> +       ATOM_PPLIB_CAC_Leakage_Record entries[] __counted_by(ucNumEntries);
>  }ATOM_PPLIB_CAC_Leakage_Table;
>
>  typedef struct _ATOM_PPLIB_PhaseSheddingLimits_Record
> @@ -568,8 +573,10 @@ 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.
> +       // Number of entries.
> +       UCHAR ucNumEntries;
> +       // Dynamically allocate entries.
> +       ATOM_PPLIB_PhaseSheddingLimits_Record entries[] __counted_by(ucNumEntries);
>  }ATOM_PPLIB_PhaseSheddingLimits_Table;
>
>  typedef struct _VCEClockInfo{
> @@ -580,8 +587,8 @@ typedef struct _VCEClockInfo{
>  }VCEClockInfo;
>
>  typedef struct _VCEClockInfoArray{
> -    UCHAR ucNumEntries;
> -    VCEClockInfo entries[1];
> +       UCHAR ucNumEntries;
> +       VCEClockInfo entries[] __counted_by(ucNumEntries);
>  }VCEClockInfoArray;
>
>  typedef struct _ATOM_PPLIB_VCE_Clock_Voltage_Limit_Record
> @@ -592,8 +599,8 @@ 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];
> +       UCHAR numEntries;
> +       ATOM_PPLIB_VCE_Clock_Voltage_Limit_Record entries[] __counted_by(numEntries);
>  }ATOM_PPLIB_VCE_Clock_Voltage_Limit_Table;
>
>  typedef struct _ATOM_PPLIB_VCE_State_Record
> @@ -604,8 +611,8 @@ typedef struct _ATOM_PPLIB_VCE_State_Record
>
>  typedef struct _ATOM_PPLIB_VCE_State_Table
>  {
> -    UCHAR numEntries;
> -    ATOM_PPLIB_VCE_State_Record entries[1];
> +       UCHAR numEntries;
> +       ATOM_PPLIB_VCE_State_Record entries[] __counted_by(numEntries);
>  }ATOM_PPLIB_VCE_State_Table;
>
>
> @@ -626,8 +633,8 @@ typedef struct _UVDClockInfo{
>  }UVDClockInfo;
>
>  typedef struct _UVDClockInfoArray{
> -    UCHAR ucNumEntries;
> -    UVDClockInfo entries[1];
> +       UCHAR ucNumEntries;
> +       UVDClockInfo entries[] __counted_by(ucNumEntries);
>  }UVDClockInfoArray;
>
>  typedef struct _ATOM_PPLIB_UVD_Clock_Voltage_Limit_Record
> @@ -638,8 +645,8 @@ 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];
> +       UCHAR numEntries;
> +       ATOM_PPLIB_UVD_Clock_Voltage_Limit_Record entries[] __counted_by(numEntries);
>  }ATOM_PPLIB_UVD_Clock_Voltage_Limit_Table;
>
>  typedef struct _ATOM_PPLIB_UVD_Table
> @@ -657,8 +664,8 @@ typedef struct _ATOM_PPLIB_SAMClk_Voltage_Limit_Record
>  }ATOM_PPLIB_SAMClk_Voltage_Limit_Record;
>
>  typedef struct _ATOM_PPLIB_SAMClk_Voltage_Limit_Table{
> -    UCHAR numEntries;
> -    ATOM_PPLIB_SAMClk_Voltage_Limit_Record entries[];
> +       UCHAR numEntries;
> +       ATOM_PPLIB_SAMClk_Voltage_Limit_Record entries[] __counted_by(numEntries);
>  }ATOM_PPLIB_SAMClk_Voltage_Limit_Table;
>
>  typedef struct _ATOM_PPLIB_SAMU_Table
> @@ -675,8 +682,8 @@ typedef struct _ATOM_PPLIB_ACPClk_Voltage_Limit_Record
>  }ATOM_PPLIB_ACPClk_Voltage_Limit_Record;
>
>  typedef struct _ATOM_PPLIB_ACPClk_Voltage_Limit_Table{
> -    UCHAR numEntries;
> -    ATOM_PPLIB_ACPClk_Voltage_Limit_Record entries[1];
> +       UCHAR numEntries;
> +       ATOM_PPLIB_ACPClk_Voltage_Limit_Record entries[] __counted_by(numEntries);
>  }ATOM_PPLIB_ACPClk_Voltage_Limit_Table;
>
>  typedef struct _ATOM_PPLIB_ACP_Table
> @@ -743,9 +750,9 @@ typedef struct ATOM_PPLIB_VQ_Budgeting_Record{
>  } ATOM_PPLIB_VQ_Budgeting_Record;
>
>  typedef struct ATOM_PPLIB_VQ_Budgeting_Table {
> -    UCHAR revid;
> -    UCHAR numEntries;
> -    ATOM_PPLIB_VQ_Budgeting_Record         entries[1];
> +       UCHAR revid;
> +       UCHAR numEntries;
> +       ATOM_PPLIB_VQ_Budgeting_Record entries[] __counted_by(numEntries);
>  } ATOM_PPLIB_VQ_Budgeting_Table;
>
>  #pragma pack()
> --
> 2.25.1
>