Re: [PATCH v3 1/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 2 potential issue(s) to consider:

New issues:
- [Medium] Memory leak of struct xe_bb in xe_migrate_vram when copying with unsupported alignment.

Pre-existing issues:
- [High] The bounce buffer allocation uses kmalloc(64) which is not guaranteed to return a 64-byte aligned pointer when SLUB debugging (redzoning) or KASAN is enabled. If it returns an unaligned pointer, the recursive call to xe_migrate_access_memory will fail the alignment check again, leading to infinite recursion and a kernel stack overflow.
--

commit 750ad7c695067571d7f0d70322bbf7d762b30994
Author: Christoph Manszewski <c.manszewski@gmail.com>

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

This commit fixes memory corruption in MATRIX_COPY mode when the pitch
is not aligned to the destination address. It updates the pitch selection
to ensure it is aligned to both the copy length and the destination address.

> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 75b83687f1b5d..7da57dc522896 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
[ ... ]
> @@ -2225,16 +2226,11 @@ static struct dma_fence *xe_migrate_vram(struct xe_migrate *m,
[ ... ]
>  	}
>  
> +	pitch = xe_migrate_copy_pitch(xe, len, dst_L0_ofs);
> +	if (!xe->info.has_mem_copy_instr &&
> +	    drm_WARN_ON(&xe->drm,
> +			(!IS_ALIGNED(len, pitch)) ||
> +			(sram_offset | vram_addr) & XE_CACHELINE_MASK))
> +		return ERR_PTR(-EOPNOTSUPP);
> +

[Severity: Medium]
Does this code leak the batch buffer?

Looking at xe_migrate_vram(), bb is allocated earlier with xe_bb_new().
If this alignment check fails, we return ERR_PTR(-EOPNOTSUPP) directly. Should
we jump to the err label to free bb using xe_bb_free(bb, NULL) instead?

>  	bb->cs[bb->len++] = MI_BATCH_BUFFER_END;
>  	update_idx = bb->len;
[ ... ]
> @@ -2538,7 +2541,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);

[Severity: High]
This is a pre-existing issue, but can the bounce buffer allocation earlier in
this function cause an infinite recursion?

When an unaligned memory access triggers the bounce buffer path in
xe_migrate_access_memory(), it calls:

    bounce = kmalloc(XE_CACHELINE_BYTES, GFP_KERNEL);

If SLUB debugging (redzoning) or KASAN is enabled, kmalloc() might not return
a 64-byte aligned pointer. If it returns an unaligned pointer, the recursive
call to xe_migrate_access_memory() checks
!IS_ALIGNED(page_offset, XE_CACHELINE_BYTES) where page_offset is derived
from the unaligned bounce buffer address.

Since the bounce buffer is unaligned, this check fails again, triggering
another allocation and recursion until the kernel stack overflows.

Should this use a mechanism that guarantees natural alignment?

>  		if (xe->info.has_mem_copy_instr)
>  			current_bytes = min_t(int, current_bytes, U16_MAX * pitch);
>  		else

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