[PATCH] drm/radeon: make -fstrict-flex-arrays=3 happy

[PATCH] drm/radeon: make -fstrict-flex-arrays=3 happy

[Alex Deucher]

The driver parses a union where the layout up through the first
array is the same, however, the array has different sizes
depending on the elements in the union.  Be explicit to
fix the UBSAN checker.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3323
Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3")
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Kees Cook <keescook@chromium.org>
---
 drivers/gpu/drm/radeon/radeon_atombios.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c b/drivers/gpu/drm/radeon/radeon_atombios.c
index bb1f0a3371ab5..10793a433bf58 100644
--- a/drivers/gpu/drm/radeon/radeon_atombios.c
+++ b/drivers/gpu/drm/radeon/radeon_atombios.c
@@ -923,8 +923,12 @@ bool radeon_get_atom_connector_info_from_supported_devices_table(struct
 		max_device = ATOM_MAX_SUPPORTED_DEVICE_INFO;
 
 	for (i = 0; i < max_device; i++) {
-		ATOM_CONNECTOR_INFO_I2C ci =
-		    supported_devices->info.asConnInfo[i];
+		ATOM_CONNECTOR_INFO_I2C ci;
+
+		if (frev > 1)
+			ci = supported_devices->info_2d1.asConnInfo[i];
+		else
+			ci = supported_devices->info.asConnInfo[i];
 
 		bios_connectors[i].valid = false;
 
-- 
2.44.0

Re: [PATCH] drm/radeon: make -fstrict-flex-arrays=3 happy

[Christian König]

Am 15.04.24 um 15:38 schrieb Alex Deucher:
> The driver parses a union where the layout up through the first
> array is the same, however, the array has different sizes
> depending on the elements in the union.  Be explicit to
> fix the UBSAN checker.
>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3323
> Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3")
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: Kees Cook <keescook@chromium.org>

Acked-by: Christian König <christian.koenig@amd.com>

But I have a bad feeling messing with that old code.

Regards,
Christian.

> ---
>   drivers/gpu/drm/radeon/radeon_atombios.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c b/drivers/gpu/drm/radeon/radeon_atombios.c
> index bb1f0a3371ab5..10793a433bf58 100644
> --- a/drivers/gpu/drm/radeon/radeon_atombios.c
> +++ b/drivers/gpu/drm/radeon/radeon_atombios.c
> @@ -923,8 +923,12 @@ bool radeon_get_atom_connector_info_from_supported_devices_table(struct
>   		max_device = ATOM_MAX_SUPPORTED_DEVICE_INFO;
>   
>   	for (i = 0; i < max_device; i++) {
> -		ATOM_CONNECTOR_INFO_I2C ci =
> -		    supported_devices->info.asConnInfo[i];
> +		ATOM_CONNECTOR_INFO_I2C ci;
> +
> +		if (frev > 1)
> +			ci = supported_devices->info_2d1.asConnInfo[i];
> +		else
> +			ci = supported_devices->info.asConnInfo[i];
>   
>   		bios_connectors[i].valid = false;
>   

Re: [PATCH] drm/radeon: make -fstrict-flex-arrays=3 happy

[Kees Cook]

On Mon, Apr 15, 2024 at 09:38:16AM -0400, Alex Deucher wrote:
> The driver parses a union where the layout up through the first
> array is the same, however, the array has different sizes
> depending on the elements in the union.  Be explicit to
> fix the UBSAN checker.
> 
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3323
> Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3")
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: Kees Cook <keescook@chromium.org>

Yup, this looks correct to me. These were trailing arrays that were not
bounds checked prior to -fstrict-flex-arrays=3:

#define ATOM_DEVICE_DFP3_INDEX                            0x00000009
...
#define ATOM_DEVICE_DFP5_INDEX                            0x0000000B
...
#define ATOM_DEVICE_RESERVEDF_INDEX                       0x0000000F
...
#define ATOM_MAX_SUPPORTED_DEVICE_INFO			  (ATOM_DEVICE_DFP3_INDEX+1)
...
#define ATOM_MAX_SUPPORTED_DEVICE			  (ATOM_DEVICE_RESERVEDF_INDEX+1)

typedef struct _ATOM_SUPPORTED_DEVICES_INFO
	...
  ATOM_CONNECTOR_INFO_I2C   asConnInfo[ATOM_MAX_SUPPORTED_DEVICE_INFO];


typedef struct _ATOM_SUPPORTED_DEVICES_INFO_2
	...
  ATOM_CONNECTOR_INFO_I2C       asConnInfo[ATOM_MAX_SUPPORTED_DEVICE];

And these arrays had different sizes: 10 vs 16

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  drivers/gpu/drm/radeon/radeon_atombios.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c b/drivers/gpu/drm/radeon/radeon_atombios.c
> index bb1f0a3371ab5..10793a433bf58 100644
> --- a/drivers/gpu/drm/radeon/radeon_atombios.c
> +++ b/drivers/gpu/drm/radeon/radeon_atombios.c
> @@ -923,8 +923,12 @@ bool radeon_get_atom_connector_info_from_supported_devices_table(struct
>  		max_device = ATOM_MAX_SUPPORTED_DEVICE_INFO;
>  
>  	for (i = 0; i < max_device; i++) {
> -		ATOM_CONNECTOR_INFO_I2C ci =
> -		    supported_devices->info.asConnInfo[i];
> +		ATOM_CONNECTOR_INFO_I2C ci;
> +
> +		if (frev > 1)
> +			ci = supported_devices->info_2d1.asConnInfo[i];
> +		else
> +			ci = supported_devices->info.asConnInfo[i];
>  
>  		bios_connectors[i].valid = false;
>  
> -- 
> 2.44.0
> 

-- 
Kees Cook