| 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
| ||
|
Date: Thu, 10 Nov 2022 11:00:41 -0500
From: Alex Deucher <alexdeucher@...il.com>
To: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@...il.com>
Cc: Harry Wentland <harry.wentland@....com>,
Leo Li <sunpeng.li@....com>,
Rodrigo Siqueira <Rodrigo.Siqueira@....com>,
Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>,
"Pan, Xinhui" <Xinhui.Pan@....com>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Rongguang Wei <weirongguang@...inos.cn>,
Slark Xiao <slark_xiao@....com>, amd-gfx@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH] [next] drm/amdgpu: Replace one-element array with
flex-array member
On Thu, Nov 10, 2022 at 10:57 AM Alex Deucher <alexdeucher@...il.com> wrote:
>
> On Wed, Nov 9, 2022 at 2:33 AM Paulo Miguel Almeida
> <paulo.miguel.almeida.rodenas@...il.com> wrote:
> >
> > One-element arrays are deprecated, and we are replacing them with
> > flexible array members instead. So, replace one-element array with
> > flexible-array member in structs _ATOM_CONNECTOR_DEVICE_TAG_RECORD,
> > _ATOM_OBJECT_GPIO_CNTL_RECORD, _ATOM_BRACKET_LAYOUT_RECORD,
> > _ATOM_BRACKET_LAYOUT_RECORD, _ATOM_LEAKAGE_VOLTAGE_OBJECT_V3,
> > _ATOM_FUSION_SYSTEM_INFO_V3, _ATOM_I2C_DATA_RECORD,
> > _ATOM_I2C_DEVICE_SETUP_INFO, _ATOM_ASIC_MVDD_INFO and refactor the
> > rest of the code accordingly. While at it, removed a redundant casting.
> >
> > Important to mention is that doing a build before/after this patch results
> > in no binary output differences.
> >
> > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
> > routines on memcpy() and help us make progress towards globally
> > enabling -fstrict-flex-arrays=3 [1].
> >
> > Link: https://github.com/KSPP/linux/issues/79
> > Link: https://github.com/KSPP/linux/issues/238
> > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
> >
> > Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@...il.com>
> > ---
> >
> > Alex, I noticed a few structs in atombios.h that were not referenced. Is
> > there any appetite for removing them? Or is that one of those cases
> > where the structs are there should one driver ever need it?
>
> A lot of userspace tools use these tables and I'd like to keep the
> headers compatible. Also, as I mentioned in the other thread, the
> atombios command tables often parse these tables when initializing the
> hardware so when you are debugging a command table, it helps to have
> the relevant structures defined so it's easier to understand what the
> command tables are doing.
Also, because this header is shared by multiple components within AMD
(linux, windows, bios, etc.), it would make things a lot easier if we
maintained compatibility.
Applied this patch. Thanks,
Alex
>
> Alex
>
>
> >
> > Ex.:
> > struct _ATOM_I2C_DATA_RECORD
> > struct _ATOM_I2C_DEVICE_SETUP_INFO
> > struct _ATOM_ASIC_MVDD_INFO
> > ---
> > .../gpu/drm/amd/display/dc/bios/bios_parser.c | 5 ++---
> > drivers/gpu/drm/amd/include/atombios.h | 16 ++++++++--------
> > 2 files changed, 10 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> > index 39dd8b2dc254..6b9e64cd4379 100644
> > --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> > +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> > @@ -2606,8 +2606,7 @@ static enum bp_result update_slot_layout_info(
> >
> > for (;;) {
> >
> > - record_header = (ATOM_COMMON_RECORD_HEADER *)
> > - GET_IMAGE(ATOM_COMMON_RECORD_HEADER, record_offset);
> > + record_header = GET_IMAGE(ATOM_COMMON_RECORD_HEADER, record_offset);
> > if (record_header == NULL) {
> > result = BP_RESULT_BADBIOSTABLE;
> > break;
> > @@ -2621,7 +2620,7 @@ static enum bp_result update_slot_layout_info(
> >
> > if (record_header->ucRecordType ==
> > ATOM_BRACKET_LAYOUT_RECORD_TYPE &&
> > - sizeof(ATOM_BRACKET_LAYOUT_RECORD)
> > + struct_size(record, asConnInfo, 1)
> > <= record_header->ucRecordSize) {
> > record = (ATOM_BRACKET_LAYOUT_RECORD *)
> > (record_header);
> > diff --git a/drivers/gpu/drm/amd/include/atombios.h b/drivers/gpu/drm/amd/include/atombios.h
> > index 55ae93c1e365..60c44a8a067f 100644
> > --- a/drivers/gpu/drm/amd/include/atombios.h
> > +++ b/drivers/gpu/drm/amd/include/atombios.h
> > @@ -4733,7 +4733,7 @@ typedef struct _ATOM_CONNECTOR_DEVICE_TAG_RECORD
> > ATOM_COMMON_RECORD_HEADER sheader;
> > UCHAR ucNumberOfDevice;
> > UCHAR ucReserved;
> > - ATOM_CONNECTOR_DEVICE_TAG asDeviceTag[1]; //This Id is same as "ATOM_DEVICE_XXX_SUPPORT", 1 is only for allocation
> > + ATOM_CONNECTOR_DEVICE_TAG asDeviceTag[]; //This Id is same as "ATOM_DEVICE_XXX_SUPPORT"
> > }ATOM_CONNECTOR_DEVICE_TAG_RECORD;
> >
> >
> > @@ -4793,7 +4793,7 @@ typedef struct _ATOM_OBJECT_GPIO_CNTL_RECORD
> > ATOM_COMMON_RECORD_HEADER sheader;
> > UCHAR ucFlags; // Future expnadibility
> > UCHAR ucNumberOfPins; // Number of GPIO pins used to control the object
> > - ATOM_GPIO_PIN_CONTROL_PAIR asGpio[1]; // the real gpio pin pair determined by number of pins ucNumberOfPins
> > + ATOM_GPIO_PIN_CONTROL_PAIR asGpio[]; // the real gpio pin pair determined by number of pins ucNumberOfPins
> > }ATOM_OBJECT_GPIO_CNTL_RECORD;
> >
> > //Definitions for GPIO pin state
> > @@ -4982,7 +4982,7 @@ typedef struct _ATOM_BRACKET_LAYOUT_RECORD
> > UCHAR ucWidth;
> > UCHAR ucConnNum;
> > UCHAR ucReserved;
> > - ATOM_CONNECTOR_LAYOUT_INFO asConnInfo[1];
> > + ATOM_CONNECTOR_LAYOUT_INFO asConnInfo[];
> > }ATOM_BRACKET_LAYOUT_RECORD;
> >
> >
> > @@ -5161,7 +5161,7 @@ typedef struct _ATOM_GPIO_VOLTAGE_OBJECT_V3
> > UCHAR ucPhaseDelay; // phase delay in unit of micro second
> > UCHAR ucReserved;
> > ULONG ulGpioMaskVal; // GPIO Mask value
> > - VOLTAGE_LUT_ENTRY_V2 asVolGpioLut[1];
> > + VOLTAGE_LUT_ENTRY_V2 asVolGpioLut[];
> > }ATOM_GPIO_VOLTAGE_OBJECT_V3;
> >
> > typedef struct _ATOM_LEAKAGE_VOLTAGE_OBJECT_V3
> > @@ -5171,7 +5171,7 @@ typedef struct _ATOM_LEAKAGE_VOLTAGE_OBJECT_V3
> > UCHAR ucLeakageEntryNum; // indicate the entry number of LeakageId/Voltage Lut table
> > UCHAR ucReserved[2];
> > ULONG ulMaxVoltageLevel;
> > - LEAKAGE_VOLTAGE_LUT_ENTRY_V2 asLeakageIdLut[1];
> > + LEAKAGE_VOLTAGE_LUT_ENTRY_V2 asLeakageIdLut[];
> > }ATOM_LEAKAGE_VOLTAGE_OBJECT_V3;
> >
> >
> > @@ -6599,7 +6599,7 @@ typedef struct _ATOM_FUSION_SYSTEM_INFO_V3
> > typedef struct _ATOM_I2C_DATA_RECORD
> > {
> > UCHAR ucNunberOfBytes; //Indicates how many bytes SW needs to write to the external ASIC for one block, besides to "Start" and "Stop"
> > - UCHAR ucI2CData[1]; //I2C data in bytes, should be less than 16 bytes usually
> > + UCHAR ucI2CData[]; //I2C data in bytes, should be less than 16 bytes usually
> > }ATOM_I2C_DATA_RECORD;
> >
> >
> > @@ -6610,14 +6610,14 @@ typedef struct _ATOM_I2C_DEVICE_SETUP_INFO
> > UCHAR ucSSChipID; //SS chip being used
> > UCHAR ucSSChipSlaveAddr; //Slave Address to set up this SS chip
> > UCHAR ucNumOfI2CDataRecords; //number of data block
> > - ATOM_I2C_DATA_RECORD asI2CData[1];
> > + ATOM_I2C_DATA_RECORD asI2CData[];
> > }ATOM_I2C_DEVICE_SETUP_INFO;
> >
> > //==========================================================================================
> > typedef struct _ATOM_ASIC_MVDD_INFO
> > {
> > ATOM_COMMON_TABLE_HEADER sHeader;
> > - ATOM_I2C_DEVICE_SETUP_INFO asI2CSetup[1];
> > + ATOM_I2C_DEVICE_SETUP_INFO asI2CSetup[];
> > }ATOM_ASIC_MVDD_INFO;
> >
> > //==========================================================================================
> > --
> > 2.37.3
> >
Powered by blists - more mailing lists