Re: [PATCH v5 22/33] drm/xe/survivability: Report 'boot status' on sysfs failure

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

Pre-existing issues:
- [High] The `csc_hw_error_work` workqueue item is never canceled during driver teardown, leading to a potential Use-After-Free.
- [High] Hardware-controlled infinite loop while traversing the auxiliary info register linked list.
--

--- Patch [22]: [PATCH v5 22/33] drm/xe/survivability: Report 'boot status' on sysfs failure ---
commit 24540c4b13c7579fcae729221e696b114366060f
Author: Michal Wajdeczko <[email protected]>

drm/xe/survivability: Report 'boot status' on sysfs failure

If we fail to create any of the survivability sysfs files, the user
can't view boot status registers any more. Report them into dmesg
instead using our existing SIGID helper.

> diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.c b/drivers/gpu/drm/xe/xe_survivability_mode.c
> index 8b74bbc963903..e14553295171d 100644
> --- a/drivers/gpu/drm/xe/xe_survivability_mode.c
> +++ b/drivers/gpu/drm/xe/xe_survivability_mode.c
[ ... ]
> @@ -329,6 +330,9 @@ static int create_survivability_sysfs(struct pci_dev *pdev)
>  
>  failed:
>  	xe_err(xe, "Failed to create survivability sysfs files: %pe\n", ERR_PTR(ret));
> +	/* no sysfs, dump Survivability info to dmesg instead */
> +	if (needs_info)
> +		log_survivability_info(xe);
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but can this loop run infinitely if the hardware
returns corrupted or circular data?

drivers/gpu/drm/xe/xe_survivability_mode.c:populate_survivability_info() {
    ...
		for (id = REG_FIELD_GET(AUXINFO_REG_OFFSET, reg_value);
		     id >= AUX_INFO0 && id < MAX_SCRATCH_REG;
		     id =  REG_FIELD_GET(AUXINFO_HISTORY_OFFSET, info[id]))
			set_survivability_info(mmio, info, id);
    ...
}

Since survivability mode handles wedged or failing firmware, the hardware might
return an AUXINFO_HISTORY_OFFSET that points to the current or a previously
visited index.

Does this traversal need a bound or cycle detection to prevent hanging a kernel
thread?

[Severity: High]
This is a pre-existing issue, but is there a potential use-after-free during
driver teardown if this work item is not canceled?

drivers/gpu/drm/xe/xe_hw_error.c:xe_hw_error_init() {
    ...
	INIT_WORK(&tile->csc_hw_error_work, csc_hw_error_work);
    ...
}

The work item is initialized without a corresponding teardown action
(like devm_add_action_or_reset) to cancel it on driver unbind.

If the driver is unloaded while hardware error processing is queued, the worker
could execute after the tile and xe structures are freed:

drivers/gpu/drm/xe/xe_hw_error.c:csc_hw_error_work() {
	struct xe_tile *tile = container_of(work, typeof(*tile), csc_hw_error_work);

	xe_hw_error_print(tile, CSC_HW_ERROR);
}

Does the driver need a cancel_work_sync() during teardown to prevent this?

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