Re: [PATCH AUTOSEL 6.18] drm/xe: Fix null pointer dereference in devcoredump cleanup

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Missing cancellation of deferred snapshot work during driver unload leads to a use-after-free.
- [High] Calling cancel_work_sync() from within the work item itself causes a self-deadlock on the dev_coredumpm_timeout() error path.
- [High] dev_coredumpm_timeout() synchronous error path clears the snapshot memory, causing a NULL pointer dereference in the worker thread.
--

commit 9607c0ac0490954ce494b7fd2f2026e574352b10
Author: Zhanjun Dong <zhanjun.dong@intel.com>
Subject: drm/xe: Fix null pointer dereference in devcoredump cleanup

In xe_devcoredump_snapshot_free(), ss->gt may be NULL when the snapshot
was never fully populated. Guard the xe_guc_capture_put_matched_nodes()
call with IS_ERR_OR_NULL() to prevent a null dereference.

Link: https://patch.msgid.link/20260326181017.2060209-1-zhanjun.dong@intel.com

> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> index 203e3038cc817..26a294f79e79f 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
[ ... ]
> @@ -255,7 +256,8 @@ static void xe_devcoredump_free(void *data)
>  	if (!data || !coredump_to_xe(coredump))
>  		return;
>  
> -	cancel_work_sync(&coredump->snapshot.work);
> +	if (coredump->captured)
> +		cancel_work_sync(&coredump->snapshot.work);

[Severity: High]
This is a pre-existing issue, but does calling cancel_work_sync() from within
the work item itself cause a self-deadlock?

In xe_devcoredump_deferred_snap_work(), which executes on the workqueue, there
is a call to dev_coredumpm_timeout(). If dev_coredumpm_timeout() fails (for
example, if devcoredump is disabled via sysfs or memory allocation fails), it
synchronously invokes the free callback, xe_devcoredump_free().

Because coredump->captured is true while this work is executing,
xe_devcoredump_free() calls cancel_work_sync() on the same work item. Since
this executes in the context of the work item itself, it blocks forever
waiting for the work to complete, deadlocking the system_unbound_wq worker.

[Severity: High]
This isn't a bug introduced by this patch, but can the synchronous error path
described above also result in a null pointer dereference within the worker
thread?

Assuming the deadlock in cancel_work_sync() is bypassed, xe_devcoredump_free()
will clear the snapshot memory:

	/* To prevent stale data on next snapshot, clear everything */
	memset(&coredump->snapshot, 0, sizeof(coredump->snapshot));

Once dev_coredumpm_timeout() returns control to
xe_devcoredump_deferred_snap_work(), the worker thread immediately attempts
to evaluate gt_to_fw(ss->gt). Since ss->gt was just zeroed out, this results
in an invalid pointer offset, which is then dereferenced by
xe_force_wake_get(), crashing the kernel.

[Severity: Critical]
This is another pre-existing issue in the devcoredump lifecycle, but is there
a risk of a use-after-free here due to missing cancellation of deferred
snapshot work during driver unload?

When the driver is unloaded or unbound, xe_driver_devcoredump_fini() executes:

static void xe_driver_devcoredump_fini(void *arg)
{
	struct drm_device *drm = arg;

	dev_coredump_put(drm->dev);
}

If a GPU hang recently queued xe_devcoredump_deferred_snap_work() on the
system workqueue, and the driver is unloaded before this work executes,
dev_coredumpm_timeout() hasn't been called yet. This means the devcd device
does not exist, so dev_coredump_put() does nothing.

The driver teardown completes and frees the xe_device memory. The workqueue
subsequently executes the pending work, dereferencing the freed coredump
structure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-170-sashal@kernel.org?part=1