Re: [PATCH v10 05/10] drm/xe/ggtt: Avoid integer overflow when validating VF GGTT range

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 9/21/2026 11:21 AM, Satyanarayana K V P wrote:
> Use check_add_overflow() for the VF range check, and rearrange the
> subsequent clamping into the subtraction form so the expression
> does not overflow.
> 
> Fixes: e904c56ba6e0 ("drm/xe: Rewrite GGTT VF initialization")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260728095026.2197333-6-satyanarayana.k.v.p%40intel.com
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Maarten Lankhorst <dev@lankhorst.se>
> ---
> V9 -> V10:
> - Cleanup old stale code added in the previous revision.
> 
> V8 -> V9:
> - Fixed review comments (Michal W).
> - Used check_add_overflow() for range checks (Michal W).
> ---
>  drivers/gpu/drm/xe/xe_ggtt.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 7cac01cef1ee..2557e51bb7cc 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -8,6 +8,7 @@
>  #include <kunit/visibility.h>
>  #include <linux/fault-inject.h>
>  #include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/overflow.h>
>  #include <linux/sizes.h>
>  
>  #include <drm/drm_drv.h>
> @@ -393,8 +394,9 @@ int xe_ggtt_init_early(struct xe_ggtt *ggtt)
>  {
>  	struct xe_device *xe = tile_to_xe(ggtt->tile);
>  	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> +	u64 ggtt_start, ggtt_size, ggtt_end;
> +	u64 wopcm = xe_wopcm_size(xe);
>  	unsigned int gsm_size;
> -	u64 ggtt_start, wopcm = xe_wopcm_size(xe), ggtt_size;
>  	int err;
>  
>  	if (!IS_SRIOV_VF(xe)) {
> @@ -408,14 +410,21 @@ int xe_ggtt_init_early(struct xe_ggtt *ggtt)
>  		}
>  		ggtt_start = wopcm;
>  		ggtt_size = (gsm_size / 8) * (u64)XE_PAGE_SIZE - ggtt_start;

nit: if we are paranoid, shouldn't we use check_mul_overflow() too?
nit: one day, we should change this magic 8 into sizeof(u64)
nit: shouldn't we check/assert that GSM covers more than just WOPCM?

then:

		if (check_mul_overflow(gsm_size / sizeof(u64),
					XE_PAGE_SIZE, &ggtt_end) ||
		    ggtt_end > GUC_GGTT_TOP) {
			ggtt_end = GUC_GGTT_TOP;
		} else if (ggtt_end < wopcm) {
			xe_tile_err(tile, "Hardware reported too small GGTT %llu\n", ..
			return -ENOSPC;
		}

  		ggtt_start = wopcm;
		ggtt_size = ggtt_end - ggtt_start;

or

	u64 gsm_to_ggtt_size(unsigned int gsm_size)
	{
		u64 ggtt_size;

		if (WARN_ONCE(check_mul_overflow(gsm_size / sizeof(u64),
						 XE_PAGE_SIZE, &ggtt_size)))
			return SZ_4G;
		return ggtt_size;
	}


		ggtt_end = min(GUC_GGTT_TOP, gsm_to_ggtt_size(gsm_size));
		if (ggtt_end < wopcm) {
			xe_tile_err(tile, "Hardware reported too small GGTT %llu\n", ..
			return -ENOSPC;
		}

  		ggtt_start = wopcm;
		ggtt_size = ggtt_end - ggtt_start;


> +
> +		if (check_add_overflow(ggtt_start, ggtt_size, &ggtt_end) ||
> +		    ggtt_end > GUC_GGTT_TOP)
> +			ggtt_size = GUC_GGTT_TOP - ggtt_start;
> +
>  	} else {
>  		ggtt_start = xe_tile_sriov_vf_ggtt_base(ggtt->tile);
>  		ggtt_size = xe_tile_sriov_vf_ggtt(ggtt->tile);
>  
>  		if (ggtt_start < wopcm ||
> -		    ggtt_start + ggtt_size > GUC_GGTT_TOP) {
> -			xe_tile_err(ggtt->tile, "Invalid GGTT configuration: %#llx-%#llx\n",
> -				    ggtt_start, ggtt_start + ggtt_size - 1);
> +		    check_add_overflow(ggtt_start, ggtt_size, &ggtt_end) ||
> +		    ggtt_end > GUC_GGTT_TOP) {
> +			xe_tile_err(ggtt->tile,
> +				    "Invalid GGTT configuration: base %#llx, size %#llx\n",
> +				    ggtt_start, ggtt_size);
>  			return -ERANGE;
>  		}
>  	}
> @@ -424,9 +433,6 @@ int xe_ggtt_init_early(struct xe_ggtt *ggtt)
>  	if (xe_vram_needs_64k(xe))
>  		ggtt->flags |= XE_GGTT_FLAGS_64K;
>  
> -	if (ggtt_size + ggtt_start > GUC_GGTT_TOP)
> -		ggtt_size = GUC_GGTT_TOP - ggtt_start;
> -
>  	if (GRAPHICS_VERx100(xe) >= 1270)
>  		ggtt->pt_ops =
>  			(ggtt->tile->media_gt && XE_GT_WA(ggtt->tile->media_gt, 22019338487)) ||