Re: [PATCH v5 2/4] drm/xe/xe_migrate: Align MEM_COPY pitch with destination address

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: negative integer overflow in xe_migrate_access_memory length chunking
--

commit 742737639ebd031db7d22e7a2865500651dcc226
Author: Christoph Manszewski <c.manszewski@gmail.com>

drm/xe/xe_migrate: Align MEM_COPY pitch with destination address

This commit modifies xe_migrate_copy_pitch() to incorporate the destination
address into pitch selection. It ensures the chosen pitch is aligned to both
the copy length and the destination address to prevent memory corruption in
MATRIX_COPY mode.

> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 3711d4542ad3d..4b995c93fcea9 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
[ ... ]
> @@ -2535,7 +2540,9 @@ int xe_migrate_access_memory(struct xe_migrate *m, struct xe_bo *bo,
>  		else
>  			current_bytes = min_t(int, bytes_left, cursor.size);
>  
> -		pitch = xe_migrate_copy_pitch(xe, current_bytes);
> +		pitch = xe_migrate_copy_pitch(xe, current_bytes,
> +					      write ? vram_addr :
> +					      (unsigned long)buf & ~PAGE_MASK);
>  		if (xe->info.has_mem_copy_instr)
>  			current_bytes = min_t(int, current_bytes, U16_MAX * pitch);

[Severity: High]
Does this code risk a negative integer overflow?

The min_t macro casts both arguments to a signed int. On systems with a
64KB PAGE_SIZE (e.g. ARM64 or PPC64), if pitch is PAGE_SIZE, U16_MAX * pitch
evaluates to 4294901760 (0xFFFE0000). When cast to int, this becomes the
negative number -131072.

This would cause current_bytes to become negative. When this negative chunk
size is passed down the call chain:

xe_migrate_access_memory()
    ...
    __fence = xe_migrate_vram(m, current_bytes, ...)

Inside xe_migrate_vram(), this negative length is implicitly cast to an
unsigned long, becoming a massive value that either triggers a kernel panic
via xe_assert(xe, npages * PAGE_SIZE <= MAX_PREEMPTDISABLE_TRANSFER), or
causes the batch buffer allocation xe_bb_new() to fail with -ENOMEM.

Note that follow-up Patch 3 in the series simply moves this broken min_t
logic into a new block, failing to resolve the signed overflow.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014142706.598490-1-jan.maslak@intel.com?part=2