lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 12 Jul 2023 10:12:08 -0400
From:   Alex Deucher <alexdeucher@...il.com>
To:     Ricardo Cañuelo <ricardo.canuelo@...labora.com>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@...il.com>
Cc:     alexander.deucher@....com, kernel@...labora.com,
        linux-kernel@...r.kernel.org, amd-gfx@...ts.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: replace 1-element arrays with flexible arrays

On Wed, Jul 12, 2023 at 8:04 AM Ricardo Cañuelo
<ricardo.canuelo@...labora.com> 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@...labora.com>
> ---
>  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
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ