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

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 20-03-2024 10:39, Dixit, Ashutosh wrote:
> 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.
sure.
>
>> +		return -ENODATA;
>> +
>> 	struct xe_device *xe = coredump_to_xe(coredump);
> Declarations after code statements are not premitted in standard C. Does
> this compile?
I was also assuming it will fail but it does pass compilation.
>
> If you fix these things looks like we're back to the original code?
Sorry I didn't understand this. Can you elaborate on this ?
>
>
>> +	/* 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
>>