Re: [PATCH 08/12] drm/xe/pat: Assert pat.ops before dereferencing members

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Ineffective bug fix / Unhandled NULL pointer dereference: The newly added `xe_assert(xe, xe->pat.ops)` does not halt execution, failing to prevent the subsequent NULL pointer dereference on `xe->pat.ops`.
--

--- Patch [8]: [PATCH 08/12] drm/xe/pat: Assert pat.ops before dereferencing members ---
commit 621e2fef21f2cbefa89f2639ce18871bffca6aa2
Author: Dnyaneshwar Bhadane <[email protected]>

drm/xe/pat: Assert pat.ops before dereferencing members

This commit attempts to catch missing PAT tables early by adding an
assertion for xe->pat.ops.

> diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c
> index a5fe1beec652d..4331ef097ba0c 100644
> --- a/drivers/gpu/drm/xe/xe_pat.c
> +++ b/drivers/gpu/drm/xe/xe_pat.c
> @@ -690,6 +690,7 @@ void xe_pat_init_early(struct xe_device *xe)
>  			GRAPHICS_VER(xe), GRAPHICS_VERx100(xe) % 100);
>  	}
>  
> +	xe_assert(xe, xe->pat.ops);

[Severity: High]
This is a pre-existing issue, but does this xe_assert() actually prevent
the subsequent NULL pointer dereference in xe_pat_init_early()?

The commit message states the assert is meant to catch a missing PAT table
"rather than faulting on the subsequent ops member checks". However,
xe_assert() wraps drm_WARN and does not return or halt execution.

If the driver is booted on an unsupported platform where the table is
missing, the preceding else branch emits an error log but leaves
xe->pat.ops as NULL. The newly added assert will emit a warning on
debug builds, but control flow will immediately fall through to the
next line and still cause a kernel panic by dereferencing the NULL pointer.

Could this function return an error code or bail out early when xe->pat.ops
is missing, instead of relying on an assertion that allows execution
to continue?

>  	xe_assert(xe, xe->pat.ops->dump);
>  	xe_assert(xe, xe->pat.ops->program_graphics);
>  	xe_assert(xe, MEDIA_VER(xe) < 13 || xe->pat.ops->program_media);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8