Re: [PATCH v3 2/5] drm/xe: Move xe_device_wedged_fini() registration to xe_device_probe_early()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unbalanced PM runtime reference leak (underflow) during PCI error recovery.
- [High] Unbounded memory accumulation (Denial of Service) via `barrier_fault` dummy page allocation.
- [High] Uninitialized work_struct (`csc_hw_error_work`) on remote tiles leads to kernel panic.
- [High] Uncanceled `csc_hw_error_work` leads to Use-After-Free during driver unload.
--

commit 372b7f6c41961551c6da41044311be66293a0cdb
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe: Move xe_device_wedged_fini() registration to xe_device_probe_early()

This commit moves the registration of the wedged cleanup action to the early
probe phase. This helps prevent a PM reference leak if the driver probe fails
after the device is declared wedged but before the cleanup was registered.

> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
[ ... ]
> @@ -838,6 +840,10 @@ int xe_device_probe_early(struct xe_device *xe)
>  	if (err)
>  		return err;
>  
> +	err = drmm_add_action_or_reset(&xe->drm, xe_device_wedged_fini, xe);
> +	if (err)
> +		return err;
> +
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but I noticed a potential unbalanced PM runtime
reference leak related to the wedged cleanup action being registered here.

In xe_pci_error_slot_reset() within drivers/gpu/drm/xe/xe_pci_error.c, the
wedged flag is forcibly set:

    atomic_set(&xe->wedged.flag, 1);

If a PCI error triggers this, the flag is set to 1 without taking a PM runtime
reference. If the device is hot-unplugged before xe_pci_error_resume() can
clear the flag, xe_device_wedged_fini() will see the flag set during teardown
and call xe_pm_runtime_put(xe).

Will this cause a PM usage counter underflow?

[Severity: High]
This is a pre-existing issue, but while looking at drmm_add_action_or_reset()
usage in this file, I noticed a potential memory accumulation in
barrier_fault() within drivers/gpu/drm/xe/xe_device.c.

When barrier_fault() allocates a dummy page, it registers a cleanup action:

    if (drmm_add_action_or_reset(dev, barrier_release_dummy_page, page))

These callbacks are tied to the drm_device lifecycle. If a device is unplugged
(causing drm_dev_enter to fail) and a local user repeatedly maps, accesses to
fault, and unmaps the barrier offset, will this leak memory until the file
descriptor is fully closed?

[Severity: High]
This is a pre-existing issue, but reviewing the hardware error paths revealed
a potential kernel panic with uninitialized work structures.

In xe_hw_error_init() within drivers/gpu/drm/xe/xe_hw_error.c, the hardware
error work is only initialized for the root tile:

    INIT_WORK(&tile->csc_hw_error_work, csc_hw_error_work);

If a CSC hardware error occurs on a remote tile, xe_hw_error_irq_handler()
attempts to schedule it:

    schedule_work(&tile->csc_hw_error_work);

Could this crash the kernel when the workqueue core manipulates the
uninitialized list on the remote tile?

[Severity: High]
This is a pre-existing issue, but there also appears to be a use-after-free
risk with this same work structure during driver unload.

The work is initialized in xe_hw_error_init() within
drivers/gpu/drm/xe/xe_hw_error.c:

    INIT_WORK(&tile->csc_hw_error_work, csc_hw_error_work);

If a CSC error schedules this work on the global workqueue, and the driver is
subsequently unbound (e.g., via hot-unplug or probe failure) before the work
executes, the xe_tile memory structure is freed.

Since there is no corresponding cancel_work_sync() during teardown, can the
worker function access a freed tile pointer when it eventually runs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907083541.2194747-1-raag.jadav@intel.com?part=2