[PATCH] drm/amdgpu: Handle potential NULL pointer dereference

[Russell, Kent]

[AMD Official Use Only - General]

Good point, as if (1 || 0) should short-circuit at the if (1) part. Thus we should go down to NOTIFY_DONE if m is NULL. Yazen can confirm here, as he was the one who with me when we found the original issue. It's possible that one of the 3 message fields was NULL instead of the actual message in our repro situation, but I'll double-check the short-circuit side of C to confirm. I know it short circuits for &&, I don't know if it does for ||.

 Kent

-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar at amd.com> 
Sent: Thursday, August 25, 2022 8:34 AM
To: Russell, Kent <Kent.Russell at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Ghannam, Yazen <Yazen.Ghannam at amd.com>
Subject: Re: [PATCH] drm/amdgpu: Handle potential NULL pointer dereference



On 8/25/2022 5:16 PM, Russell, Kent wrote:
> [AMD Official Use Only - General]
> 
> Friendly ping
> 

Wonder how it goes any further when m is NULL. It should do shortcut evaluation and return NOTIFY_DONE, right? Or is this for better readability?

Thanks,
Lijo

>   Kent
> 
> -----Original Message-----
> From: Russell, Kent <Kent.Russell at amd.com>
> Sent: Monday, August 15, 2022 11:31 AM
> To: amd-gfx at lists.freedesktop.org
> Cc: Ghannam, Yazen <Yazen.Ghannam at amd.com>; Russell, Kent 
> <Kent.Russell at amd.com>
> Subject: [PATCH] drm/amdgpu: Handle potential NULL pointer dereference
> 
> If m is NULL, we will end up referencing a NULL pointer in the subsequent m elements like extcpu, bank and status. Pull the NULL check out and do it first before referencing m's elements.
> 
> Signed-off-by: Kent Russell <kent.russell at amd.com>
> ---
>   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 ab9ba5a9c33d..028495fdfa62 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -2838,12 +2838,15 @@ static int amdgpu_bad_page_notifier(struct notifier_block *nb,
>   	struct eeprom_table_record err_rec;
>   	uint64_t retired_page;
>   
> +	if (!m)
> +		return NOTIFY_DONE;
> +
>   	/*
>   	 * If the error was generated in UMC_V2, which belongs to GPU UMCs,
>   	 * and error occurred in DramECC (Extended error code = 0) then only
>   	 * process the error, else bail out.
>   	 */
> -	if (!m || !((smca_get_bank_type(m->extcpu, m->bank) == SMCA_UMC_V2) &&
> +	if (!((smca_get_bank_type(m->extcpu, m->bank) == SMCA_UMC_V2) &&
>   		    (XEC(m->status, 0x3f) == 0x0)))
>   		return NOTIFY_DONE;
>   
> --
> 2.25.1
>