Re: [PATCH v2 25/33] drm/xe: Preallocate system BO backing outside the dma-resv lock

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Integer truncation in xe_device_ccs_bytes() returning u32 causes buffer overflows and physical memory corruption/panics for BOs >= 1TB.
--

--- Patch [25]: [PATCH v2 25/33] drm/xe: Preallocate system BO backing outside the dma-resv lock ---
commit aa051bafd6b83b01e59b139bb60870dfa1044d22
Author: Matthew Brost <matthew.brost@intel.com>
Subject: drm/xe: Preallocate system BO backing outside the dma-resv lock

xe_gem_create_ioctl() populates a buffer object's backing store inside
xe_validation_guard(), i.e. under the dma-resv lock. This patch uses the
TTM full-preallocation mechanism to move that work out of the lock.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index a010042653495..d9226a3995fe4 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
[ ... ]
> @@ -3994,6 +4009,45 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>  			return -ENOENT;
>  	}
>  
> +	/*
> +	 * For system-only BOs, preallocate the whole backing outside the
> +	 * dma-resv lock so the populate under the validation guard does not
> +	 * stall in reclaim/compaction. The caching mirrors the system-BO path
> +	 * in xe_ttm_tt_create(); the pool falls back to in-line allocation for
> +	 * any shortfall, so a failed fill is harmless.
> +	 */
> +	if ((bo_flags & XE_BO_FLAG_SYSTEM) &&
> +	    !(bo_flags & (XE_BO_FLAG_DEFER_BACKING | XE_BO_FLAG_VRAM_MASK |
> +			  XE_BO_FLAG_STOLEN))) {
> +		enum ttm_caching caching = ttm_cached;
> +		unsigned long num_pages = args->size >> PAGE_SHIFT;
> +
> +		if (!IS_DGFX(xe) &&
> +		    args->cpu_caching == DRM_XE_GEM_CPU_CACHING_WC)
> +			caching = ttm_write_combined;
> +
> +		/*
> +		 * Cover the CCS backup pages appended to the tt (see
> +		 * xe_ttm_tt_create()) so the whole tt is preallocated.
> +		 */
> +		if (__xe_bo_needs_ccs_pages(xe, ttm_bo_type_device, bo_flags,
> +					    args->cpu_caching))
> +			num_pages += DIV_ROUND_UP(xe_device_ccs_bytes(xe,
> +								      args->size),
> +						  PAGE_SIZE);

[Severity: High]
This is a pre-existing issue, but can this lead to an integer truncation for
very large buffer sizes?

The function xe_device_ccs_bytes() in drivers/gpu/drm/xe/xe_device.c returns
a u32, but takes a u64 size parameter. If a user requests a BO allocation of
>= 1TB via DRM_IOCTL_XE_GEM_CREATE with CCS enabled, the required CCS size
would be 1TB / 256 = 4GB, which silently truncates to 0 in the u32 return
type.

Because this patch propagates the pattern by calling xe_device_ccs_bytes() on
the user-controlled args->size, the extra_pages for the CCS backup could be
calculated as 0, under-allocating the BO's SG table. Later, when
xe_migrate_copy() processes the BO in chunks, it could advance the CCS cursor
past the end of the under-allocated SG table, resulting in a NULL pointer
dereference and kernel panic.

Should xe_device_ccs_bytes() be updated to return a u64, or should there be
size limits enforced earlier in the ioctl?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710215442.2444235-1-matthew.brost@intel.com?part=25