RE: [PATCH] drm/amdgpu: fix UBSAN array-index-out-of-bounds for ras_block_string[]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@xxxxxxx> 
Sent: Tuesday, January 16, 2024 7:16 PM
To: Wang, Yang(Kevin) <KevinYang.Wang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Zhang, Hawking <Hawking.Zhang@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: fix UBSAN array-index-out-of-bounds for ras_block_string[]

On 1/16/2024 4:32 PM, Yang Wang wrote:
> fix array index out of bounds issue for ras_block_string[] array.
> 
> Fixes: 2e3675fe4e3ee ("drm/amdgpu: Align ras block enum with 
> firmware")
> 
> Signed-off-by: Yang Wang <kevinyang.wang@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index ff6f84714f68..8004863719d0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -74,6 +74,8 @@ const char *ras_block_string[] = {
>   	"mca",
>   	"vcn",
>   	"jpeg",
> +	"ih",
> +	"mpio",
>   };
>   
>   const char *ras_mca_block_string[] = { @@ -95,7 +97,8 @@ const char 
> *get_ras_block_str(struct ras_common_if *ras_block)
>   	if (!ras_block)
>   		return "NULL";
>   
> -	if (ras_block->block >= AMDGPU_RAS_BLOCK_COUNT)
> +	if (ras_block->block >= AMDGPU_RAS_BLOCK_COUNT ||
> +	    ras_block->block >= ARRAY_SIZE(ras_block_string))

Better to keep another condition for this check and return a dynamic string like "<block>_unnamed" or "<block>_nodesc"

Thanks,
Lijo


[kevin]:

The function is not allowed to return a dynamic string, and many codes rely on this constant string.
But your suggestion is good, but the purpose of this patch is only to bug fix.

Best Regards,
Kevin
>   		return "OUT OF RANGE";
>   
>   	if (ras_block->block == AMDGPU_RAS_BLOCK__MCA)





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux