| 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 10:57:58 -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 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.
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