[PATCH] drm/amdgpu/pptable: Fix UBSAN array-index-out-of-bounds
Tasos Sahanidis
tasos at tasossah.com
Mon May 27 09:42:36 UTC 2024
On 2024-05-23 17:52, Alex Deucher wrote:
> On Thu, May 23, 2024 at 9:05 AM Tasos Sahanidis <tasos at tasossah.com> wrote:
>>
>> Dyanmically sized arrays used [1] instead of []. Replacing the former
>> with the latter resolves multiple warnings observed on boot with a
>> BONAIRE card.
>>
>> Signed-off-by: Tasos Sahanidis <tasos at tasossah.com>
>> ---
>> drivers/gpu/drm/amd/include/pptable.h | 24 ++++++++++++------------
>> 1 file changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/include/pptable.h b/drivers/gpu/drm/amd/include/pptable.h
>> index 2e8e6c9875f6..d1dec880d2d6 100644
>> --- a/drivers/gpu/drm/amd/include/pptable.h
>> +++ b/drivers/gpu/drm/amd/include/pptable.h
>> @@ -480,7 +480,7 @@ typedef struct _StateArray{
>> //how many states we have
>> UCHAR ucNumEntries;
>>
>> - ATOM_PPLIB_STATE_V2 states[1];
>> + ATOM_PPLIB_STATE_V2 states[];
>
> Can you add __counted_by(ucNumEntries) to the end of the line? E.g.,
>
> ATOM_PPLIB_STATE_V2 states[] __counted_by(ucNumEntries);
>
> Same comment for the other changes below.
>
> Alex
Sure thing! I have the v2 ready and will submit after testing it on
hardware. I do have a question though. The following already uses [].
Would it be acceptable to also modify it in the same patch?
@@ -658,7 +658,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[];
+ ATOM_PPLIB_SAMClk_Voltage_Limit_Record entries[] __counted_by(numEntries);
}ATOM_PPLIB_SAMClk_Voltage_Limit_Table;
typedef struct _ATOM_PPLIB_SAMU_Table
There's also this, which I think __counted_by can be used here as well:
diff --git a/drivers/gpu/drm/amd/include/pptable.h b/drivers/gpu/drm/amd/include/pptable.h
index febc853d2a07..341d4a4e8d98 100644
--- a/drivers/gpu/drm/amd/include/pptable.h
+++ b/drivers/gpu/drm/amd/include/pptable.h
@@ -497,15 +497,15 @@ typedef struct _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;
- ATOM_PPLIB_NONCLOCK_INFO nonClockInfo[];
+ ATOM_PPLIB_NONCLOCK_INFO nonClockInfo[] __counted_by(ucNumEntries);
}NonClockInfoArray;
typedef struct _ATOM_PPLIB_Clock_Voltage_Dependency_Record
{
USHORT usClockLow;
UCHAR ucClockHigh;
USHORT usVoltage;
All the other ones are UCHAR, so __counted_by can not be used on them.
Please let me know what you think.
Thanks!
--
Tasos
More information about the amd-gfx
mailing list