[PATCH v4] drm/xe/xe_devcoredump: Check NULL before dereferencing

[Matt Roper]

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.


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