Re: [PATCH 2/2] drm/xe/guc: Sanity check GuC-reported hwconfig table size

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Integer overflow in hwconfig table parsing can lead to an infinite loop.
--

commit 7ddfaaa1b670c316c0100635679a9a76370d95cf
Author: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
drm/xe/guc: Sanity check GuC-reported hwconfig table size

This patch bounds-checks the hardware configuration table size reported by
the GuC firmware. It enforces a 4K upper limit to prevent allocating overly
large buffers based on untrusted firmware data.

> diff --git a/drivers/gpu/drm/xe/xe_guc_hwconfig.c b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> index b300901dbb8e1..fb8caf6e9240f 100644
> --- a/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> +++ b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
[ ... ]
> +/*
> + * The hwconfig table is a small KLV blob, but its length is reported by the
> + * GuC in the 28-bit DATA0 field of the MMIO response, i.e. it can claim up to
> + * 256MB. Since the reported size drives both a GGTT-pinned BO allocation that
> + * lives for the whole device lifetime and several kzalloc()s in the readers,
> + * sanity check it against a generous upper bound instead of trusting the
> + * firmware value blindly.
> + */

[Severity: High]
This is a pre-existing issue, but since the patch explicitly treats the
firmware data as untrusted, can malformed hwconfig table contents still cause
an infinite loop in the parsers?

In drivers/gpu/drm/xe/xe_guc_hwconfig.c:xe_guc_hwconfig_dump(), the bounds
check performs an addition that can wrap around:

	while (i + 3 <= num_dw) {
		u32 attribute = hwconfig[i++];
		u32 len_dw = hwconfig[i++];

		if (i + len_dw > num_dw) {

If corrupted firmware provides a large value like len_dw = 0xFFFFFFFE, the
addition i + len_dw wraps around to 0. This bypasses the bounds check since
0 > num_dw evaluates to false.

The loop index is then advanced:

		i += len_dw;
	}

Because i is an int, advancing it by 0xFFFFFFFE wraps it back to 0, which
causes the loop to repeat indefinitely and hang the kernel thread.

A similar overflow appears to be possible in xe_guc_hwconfig_lookup_u32():

	while (i + 3 <= num_dw) {
		u32 key = hwconfig[i++];
		u32 len_dw = hwconfig[i++];

		if (key != attribute) {
			i += len_dw;
			continue;
		}

Does this code need safe addition checks when handling len_dw from the
firmware table to prevent these integer overflows?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908231854.1218934-4-umesh.nerlige.ramappa@intel.com?part=2