Re: [PATCH 5.10 1/1] drm/amdkfd: Check for null pointer after calling kmemdup

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Jan 12, 2023 at 04:26:45PM +0100, Daniel Vetter wrote:
> On Thu, 12 Jan 2023 at 13:47, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Wed, Jan 04, 2023 at 07:56:33PM +0200, Dragos-Marian Panait wrote:
> > > From: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> > >
> > > [ Upstream commit abfaf0eee97925905e742aa3b0b72e04a918fa9e ]
> > >
> > > As the possible failure of the allocation, kmemdup() may return NULL
> > > pointer.
> > > Therefore, it should be better to check the 'props2' in order to prevent
> > > the dereference of NULL pointer.
> > >
> > > Fixes: 3a87177eb141 ("drm/amdkfd: Add topology support for dGPUs")
> > > Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> > > Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> > > Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > > Signed-off-by: Dragos-Marian Panait <dragos.panait@windriver.com>
> > > ---
> > >  drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> > > index 86b4dadf772e..02e3c650ed1c 100644
> > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> > > @@ -408,6 +408,9 @@ static int kfd_parse_subtype_iolink(struct crat_subtype_iolink *iolink,
> > >                       return -ENODEV;
> > >               /* same everything but the other direction */
> > >               props2 = kmemdup(props, sizeof(*props2), GFP_KERNEL);
> > > +             if (!props2)
> > > +                     return -ENOMEM;
> >
> > Not going to queue this up as this is a bogus CVE.
> 
> Are we at the point where CVE presence actually contraindicates
> backporting?

Some would say that that point passed a long time ago :)

> At least I'm getting a bit the feeling there's a surge of
> automated (security) fixes that just don't hold up to any scrutiny.

That has been happening a lot more in the past 6-8 months than in years
past with the introduction of more automated tools being present.

> Last week I had to toss out an fbdev locking patch due to static
> checker that has no clue at all how refcounting works, and so
> complained that things need more locking ... (that was -fixes, but
> would probably have gone to stable too if I didn't catch it).
> 
> Simple bugfixes from random people was nice when it was checkpatch
> stuff and I was fairly happy to take these aggressively in drm. But my
> gut feeling says things seem to be shifting towards more advanced
> tooling, but without more advanced understanding by submitters. Does
> that holder in other areas too?

Again, yes, I have seen that a lot recently, especially with regards to
patches that purport to fix bugs yet obviously were never tested.

That being said, there are a few developers who are doing great things
with fault-injection testing and providing good patches for that.  So we
can't just say that everyone using these tools has no clue.

thanks,

greg k-h