Re: [PATCH] gpu/drm/xe/xe_devcoredump: Check NULL before dereferencing coredump.

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 19 Mar 2024 22:17:19 -0700, Himal Prasad Ghimiray wrote:
>
> Derefernce coredump to get xe_devcoredump_snapshot only if coredump is
> not NULL.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_devcoredump.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> index 0fcd30680323..83a1f3cd37cf 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> @@ -74,17 +74,20 @@ static ssize_t xe_devcoredump_read(char *buffer, loff_t offset,
>				   size_t count, void *data, size_t datalen)
>  {
>	struct xe_devcoredump *coredump = data;
> +	if (!coredump)

Needs empty line before this line.

> +		return -ENODATA;
> +
>	struct xe_device *xe = coredump_to_xe(coredump);

Declarations after code statements are not premitted in standard C. Does
this compile?

If you fix these things looks like we're back to the original code?


> +	/* Our device is gone already... */
> +	if (!xe)
> +		return -ENODEV;
> +
>	struct xe_devcoredump_snapshot *ss = &coredump->snapshot;
>	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))
> -		return -ENODEV;
> -
>	/* Ensure delayed work is captured before continuing */
>	flush_work(&ss->work);
>
> --
> 2.25.1
>