Re: [PATCH v3 02/13] drm/xe/ggtt: Report GGTT errors using structured logging

[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 VF GGTT bounds check allows a compromised PF/GuC to cause out-of-bounds MMIO writes in the VF driver.
--

commit 835ff291efae84e92f4b4664f3d531bd0f480f5d
Author: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>

drm/xe/ggtt: Report GGTT errors using structured logging

This commit replaces xe_tile_err() calls with xe_log_err_fatal() using
the GGTT component tag in GGTT initialization error paths.

> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 63e8bf1936058..773777afc68a2 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c

[ ... ]

> @@ -414,8 +416,9 @@ int xe_ggtt_init_early(struct xe_ggtt *ggtt)
>  
>  		if (ggtt_start < wopcm ||
>  		    ggtt_start + ggtt_size > GUC_GGTT_TOP) {

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the
bounds check to be bypassed by an integer overflow?

If a compromised PF (host) or GuC firmware returns a maliciously crafted
ggtt_start and massive ggtt_size via the GuC KLV query, the addition of
ggtt_start + ggtt_size could overflow a 64-bit unsigned integer and wrap
around to a small value.

This would bypass the > GUC_GGTT_TOP check, allowing the unmodified
massive size to be passed to drm_mm_init() and could subsequently lead
to out-of-bounds MMIO writes in the guest kernel.

> -			xe_tile_err(ggtt->tile, "Invalid GGTT configuration: %#llx-%#llx\n",
> -				    ggtt_start, ggtt_start + ggtt_size - 1);
> +			xe_log_err_fatal(ggtt->tile, GGTT, -ERANGE,
> +					 "Invalid GGTT configuration: %#llx-%#llx\n",
> +					 ggtt_start, ggtt_start + ggtt_size - 1);
>  			return -ERANGE;
>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908104550.1283933-1-dnyaneshwar.bhadane@intel.com?part=2