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

[Russell, Kent]

[AMD Official Use Only - General]



> -----Original Message-----
> From: Ghannam, Yazen <Yazen.Ghannam at amd.com>
> Sent: Friday, August 26, 2022 11:20 AM
> To: Russell, Kent <Kent.Russell at amd.com>
> Cc: Lazar, Lijo <Lijo.Lazar at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: Handle potential NULL pointer dereference
> 
> On Thu, Aug 25, 2022 at 08:54:46AM -0400, Russell, Kent wrote:
> > [AMD Official Use Only - General]
> >
> > It does indeed short-circuit on || (If the left side of an || statement is not 0, it
> doesn't evaluate the right side and returns true). So we can ignore this patch,
> since checking for each individual field on the 2nd term is probably overkill. We
> were still investigating what got passed in and why it wasn't valid, so I'll drop this
> for now. Thanks Lijo!
> 
> Right, the "||" will short-circuit, but not the "&&".
> 
> ...
> 
> > > +	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)))
> 
> If m is NULL, then we get (!m || "Don't care") = true. We avoid the second
> operand.
> 
> But then we have (true && "Something with 'm->' in it"). The second operand in
> the && needs to be checked, and this will cause a NULL pointer dereference if
> m is NULL.
> 
> What do you guys think?
> 

Since the 2nd field is in parentheses, it would be
whatever = (smca_get_bank_type && XEC)
if (!m || !(whatever))

In that case we should still jump out at !m, since the 2nd term is completely contained in that !((...)) clump. At least that's how I understand it after Lijo's original comments.

 Kent


> Thanks,
> Yazen