Re: [PATCH v5 20/33] drm/xe/survivability: Report 'boot status' using SIGID

[email protected]
Newsgroups org.freedesktop.lists.intel-xe
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential infinite loop in `populate_survivability_info()` due to unvalidated hardware-provided linked list.
--

--- Patch [20]: [PATCH v5 20/33] drm/xe/survivability: Report 'boot status' using SIGID ---
commit 777c3378750db72921a668da05900b90d4fd8d8e
Author: Michal Wajdeczko <[email protected]>

drm/xe/survivability: Report 'boot status' using SIGID

This commit updates the driver to report 'boot status' and other scratch
register details using the xe_log_info() macros. It also adds a helper to
decode the boot status code and changes the pdev parameter to xe.

> diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.c b/drivers/gpu/drm/xe/xe_survivability_mode.c
> index 4c506027fa94c..0ace10ae500af 100644
> --- a/drivers/gpu/drm/xe/xe_survivability_mode.c
> +++ b/drivers/gpu/drm/xe/xe_survivability_mode.c
[ ... ]
> @@ -172,18 +173,32 @@ static void populate_survivability_info(struct xe_device *xe)
>  	}
>  }

[Severity: High]
This is a pre-existing issue, but is there a risk of an infinite loop in
populate_survivability_info() if the hardware returns a cyclic linked list?

Looking at the loop that traverses the aux info registers just above this
point in the function:

	if (reg_value & AUXINFO_SUPPORT) {
		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);
	}

If the PCODE firmware returns a value in AUXINFO_HISTORY_OFFSET that points
to the current register index or creates a cycle among the registers, could
this loop run indefinitely without a limit to the iteration count? 

An infinite loop here might result in a kernel thread hang during device
probe.

> -static void log_survivability_info(struct pci_dev *pdev)
> +static const char *boot_status_str(u8 boot_status)
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=20
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.