[PATCH v4] drm/xe/xe_devcoredump: Check NULL before dereferencing
Ghimiray, Himal Prasad
himal.prasad.ghimiray at intel.com
Thu Mar 28 12:11:24 UTC 2024
> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: 25 March 2024 22:39
> To: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>
> Cc: intel-xe at lists.freedesktop.org; Dixit, Ashutosh <ashutosh.dixit at intel.com>;
> Souza, Jose <jose.souza at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>
> Subject: Re: [PATCH v4] drm/xe/xe_devcoredump: Check NULL before
> dereferencing
>
> On Fri, Mar 22, 2024 at 10:54:47PM +0530, Himal Prasad Ghimiray wrote:
> > Dereference 'coredump' to access 'xe_devcoredump_snapshot' only if
> > 'coredump' is not NULL,
>
> There wasn't actually any NULL dereference here, right?
> coredump_to_xe(coredump) is just container_of pointer math and doesn't
> dereference the pointer. Likewise, &coredump->snapshot is also fine for NULL
> pointers.
>
> The change looks fine as a general cleanup / future-proofing, the title just seems
> misleading since it makes it sound like there was a bug being fixed.
Hi Matt,
Yes you are right. Will modify the commit message accordingly.
BR
Himal
>
>
> Matt
>
> >
> > v2
> > - Fix commit messages.
> >
> > v3
> > - Define variables before code.(Ashutosh/Jose)
> >
> > v4
> > - Drop return check for coredump_to_xe. (Jose/Rodrigo)
> >
> > Cc: Ashutosh Dixit <ashutosh.dixit at intel.com>
> > Cc: José Roberto de Souza <jose.souza at intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_devcoredump.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c
> > b/drivers/gpu/drm/xe/xe_devcoredump.c
> > index 3a6263ecff01..a951043b2943 100644
> > --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> > +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> > @@ -77,17 +77,19 @@ static ssize_t xe_devcoredump_read(char *buffer,
> loff_t offset,
> > size_t count, void *data, size_t datalen) {
> > struct xe_devcoredump *coredump = data;
> > - struct xe_device *xe = coredump_to_xe(coredump);
> > - struct xe_devcoredump_snapshot *ss = &coredump->snapshot;
> > + struct xe_device *xe;
> > + struct xe_devcoredump_snapshot *ss;
> > struct drm_printer p;
> > struct drm_print_iterator iter;
> > struct timespec64 ts;
> > int i;
> >
> > - /* Our device is gone already... */
> > - if (!data || !coredump_to_xe(coredump))
> > + if (!coredump)
> > return -ENODEV;
> >
> > + xe = coredump_to_xe(coredump);
> > + ss = &coredump->snapshot;
> > +
> > /* Ensure delayed work is captured before continuing */
> > flush_work(&ss->work);
> >
> > --
> > 2.25.1
> >
>
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation
More information about the Intel-xe
mailing list