Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <04e37ee1-53b0-97ab-d6d7-a39edfbdc2ea@amd.com>
Date:   Fri, 28 Oct 2022 09:18:39 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH] [next] drm/amdgpu: Replace one-element array with
 flexible-array member
Content-Language: en-US
To:     Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Hans de Goede <hdegoede@redhat.com>,
        Grigory Vasilyev <h0tc0d3@gmail.com>,
        Claudio Suarez <cssk@net-c.es>,
        Slark Xiao <slark_xiao@163.com>,
        Rongguang Wei <weirongguang@kylinos.cn>,
        amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc:     linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
References: <Y1tkWdwPUp+UdpM0@mail.google.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <Y1tkWdwPUp+UdpM0@mail.google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VHVEaENrRkJIMXdxZGYxUVRpRVBnY2h0TUZCT0lmREdvZHNlaE9NaWhKSWNw?=
 =?utf-8?B?dG10dGdpQXZ2VVJsazlkUkxjVDZleEtWMVNtdkIycnE2dXczNktyZ0ZTNENs?=
 =?utf-8?B?OExOeEtYYXl4Y25zS2JsTTdKbW9tMXI2UUxIQUtGa1U5QWQ3VktVNGFhS2lG?=
 =?utf-8?B?VW90ZSt5WlJLVmFGTytyQUJXQ09mQWh3ZytadGZ4SEx3RmtWaHZKd0pTZmJE?=
 =?utf-8?B?UmhNZHNjNDJYcExYUXpKempNK3dXSVhVdEx4VjBGaFRmNEdESGcvUk14eFpv?=
 =?utf-8?B?YU5jenRpYmtPeXZ5NUtXT01ja3J0ZFJKYUo4c0RyRVBnd0tkMENRODB2Q2g4?=
 =?utf-8?B?WUhrc2ZOSzFlTTVNb3QwWVcyMEM1UXJPRmNWaVJtWFJIYUl1N0EyVDU3Yzhu?=
 =?utf-8?B?SFpHSnliUkpsVkNxcHNWWFdNK0VOa1pLYlVjSHU2NzdCd1RjUTF4NkprUENr?=
 =?utf-8?B?THY3d0wxNTJyY1BZQnF3ZmsyUktFa3c0TVYvcG1ybkg5cTVjQ3NKMXVBb3lx?=
 =?utf-8?B?by8wSysxVWpYM0dHN2FQOUJsZ1dBelRmS2xXdFBOcnBxQUx5Ujc2aFJ6TVdr?=
 =?utf-8?B?dXMrRk9mbVM0ajJzTXQ4MXVsakNTLzV0cUtlNVlpc3QvTkhkOElSZWNocldt?=
 =?utf-8?B?NkZuazVlVGNWTDI1TTQ1NGFqVk4vTGFhemQvZGJ3Y3l6ZEhGenFYdkFJRGhN?=
 =?utf-8?B?dE1VYkRlWXRNZzlpVklQL2FRR0ZkNUc3Mm1HUjlpOG8zSGtsRUM1ejlRc21U?=
 =?utf-8?B?WTdNaU9FQkNkcHJUbVpCZjdyWGNLbHlCa3JmL2FPanpaVGZTbmtMZTl1UFdZ?=
 =?utf-8?B?T24ycE5nc3g5UjRwcEs3TklFU1FIRjN3andEb3pLZm9jZzVMblFqL0Y0WUI4?=
 =?utf-8?B?QXNvbnRPVVNoRE93RFErRldGSmVVdlpLYmxnVXNuOGVBTmxuZXA2ZHY0bjcr?=
 =?utf-8?B?bW5OaHhYajdibjNuZWRZOXY0TUFkcGxZUmhHMTVYUHhwUDAzc1ByeDNQOHdm?=
 =?utf-8?B?Vyt1Vkk5SWp0NTJHak1TSFZRN2pJZHMrOHkxSVhIZ1VGdU9PdEpueVdxZHRr?=
 =?utf-8?B?ODZXeXZMWk5ZbHA4WVhoMkE3NndNSG11djAwMFZjL1ZWZkx2WDl4SFV0ZHE4?=
 =?utf-8?B?b1ViY3IvQWt1eE5IcFk2UnRWTzZkajl0c2RGa0l2TkRIYUxCUWJCckR6WGVv?=
 =?utf-8?B?enMzMnZpQWloM1JYQkcwOWFPMGhLRUNpTmNLS3psbnoxdjlidW5vL29BM0VG?=
 =?utf-8?B?WEYydTFJbW9OSVY2ZUdlN2JXbE81T3RiNlZIcjVkdnhRenlXNDUzMHhyanpx?=
 =?utf-8?B?T2ZqVytJSFFScm0xUVNQTVFYK2hKb1pTYXRVWE9acmN1M1dFd1RkaTJLKzVF?=
 =?utf-8?B?S2wvUHJPNDdQV3VNc1pnak9DT1ZaR1c5U3djUHpvQ2tyTG00VGdkK3JJTG5v?=
 =?utf-8?B?dFJyQS9kUldjOHNoUGg1cFd5THhnc3pzZG84N1ZRakFjNE81T3lPRnUyZUdP?=
 =?utf-8?B?R1gxZEJ2VGllVHVrc3VxaEhXZXFKTFUxdUgrT3RNUFFWaTBTc1RTMUtkSlhl?=
 =?utf-8?B?RTZnWGMzK2VuVTliSGFnRmFibTBRQy9lMlZhUHNYY2ZxcXhLWnRGc1N2dWxw?=
 =?utf-8?B?YzJxN05kVXlJVHZ4MndJbHZYekxReWFIZEUxM1Z5UlBFTXF3Q290Y1FTWmd2?=
 =?utf-8?B?Uy9PZ2tVcGN3cHErZ1hMeW9LdURjdzhGd2wxbkZGWlJlWDdGcGlka0d0cFli?=
 =?utf-8?B?blh1eEhURjhXbDArUnY2a1hkU0ZSTnJPR3lVdTVVQzlaSFVEbDZ6L3cxczhL?=
 =?utf-8?B?cW9VbDh2QjNlWDlsN2JyaTY5UzJ4OTgzYjVhdDVZVlJPMmFwWFBURU9OSzgw?=
 =?utf-8?B?NTNQQ3VVbzFzQkUvT3JTc0FkT2xLdVZTajZVcDE0OUxBTGxBL2ZoYjNSMVda?=
 =?utf-8?B?VXhPdnJIampINUFvN05QZElWVTRKUDR0M25NdSs0SjVtcllvVTFVbDVqREF1?=
 =?utf-8?B?SkRjQkU1UXF0eVRuMHgzMzlqeGpxRW82N0F5eGk5SWU4QzhHSGhOWXRoZmRH?=
 =?utf-8?B?Rlh4aWdXMFI0K1R6VEdKTmhOZlpYd2lFQlE1WGk2R2I4QTZNOWFnU0VUY2I5?=
 =?utf-8?B?UUVWUm9SQ0Ywa1JwRGhuSFpGbWNzcGZPQTY0TVJySFFXOFhESVo1Yi95SGkv?=
 =?utf-8?Q?1GDWwS8mDjYfbOb3tw5S70j8HJZQgmMwjFnv8wVhyiVD?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 73b92184-8cc0-437f-a39b-08dab8b4a5f2
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Oct 2022 07:18:44.6768
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: iK9kXa+bTlsSONqkfS9PzWou5oWz+vx+BLU+dJI7HQ87bzTDUWbIrdLxZ1SsgiIx
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR12MB5664
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 28.10.22 um 07:10 schrieb Paulo Miguel Almeida:
> 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 struct _ATOM_FAKE_EDID_PATCH_RECORD and
> refactor the rest of the code accordingly.
>
> 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]

I'm not sure if that's a good idea. We had multiple attempts to refactor 
this now and it always caused a regression.

Additional to that the header in question came from our BIOS team and 
isn't following Linux styles in general.

Alex what do you think?

Regards,
Christian.

>
> Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 10 +++++++---
>   drivers/gpu/drm/amd/include/atombios.h         |  2 +-
>   2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
> index 6be9ac2b9c5b..6b5abf1249db 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
> @@ -2079,10 +2079,14 @@ amdgpu_atombios_encoder_get_lcd_info(struct amdgpu_encoder *encoder)
>   							} else
>   								kfree(edid);
>   						}
> +
> +						record += struct_size(fake_edid_record,
> +								      ucFakeEDIDString,
> +								      fake_edid_record->ucFakeEDIDLength);
> +					} else {
> +						/* empty fake edid record must be 3 bytes long */
> +						record += sizeof(ATOM_FAKE_EDID_PATCH_RECORD) + 1;
>   					}
> -					record += fake_edid_record->ucFakeEDIDLength ?
> -						fake_edid_record->ucFakeEDIDLength + 2 :
> -						sizeof(ATOM_FAKE_EDID_PATCH_RECORD);
>   					break;
>   				case LCD_PANEL_RESOLUTION_RECORD_TYPE:
>   					panel_res_record = (ATOM_PANEL_RESOLUTION_PATCH_RECORD *)record;
> diff --git a/drivers/gpu/drm/amd/include/atombios.h b/drivers/gpu/drm/amd/include/atombios.h
> index 15943bc21bc5..b5b1d073f8e2 100644
> --- a/drivers/gpu/drm/amd/include/atombios.h
> +++ b/drivers/gpu/drm/amd/include/atombios.h
> @@ -4107,7 +4107,7 @@ typedef struct _ATOM_FAKE_EDID_PATCH_RECORD
>   {
>     UCHAR ucRecordType;
>     UCHAR ucFakeEDIDLength;       // = 128 means EDID length is 128 bytes, otherwise the EDID length = ucFakeEDIDLength*128
> -  UCHAR ucFakeEDIDString[1];    // This actually has ucFakeEdidLength elements.
> +  UCHAR ucFakeEDIDString[];     // This actually has ucFakeEdidLength elements.
>   } ATOM_FAKE_EDID_PATCH_RECORD;
>   
>   typedef struct  _ATOM_PANEL_RESOLUTION_PATCH_RECORD