Re: [PATCH v4 3/4] drm/xe: Bump prefetch BO LRU via GPUSVM devmem callback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: kernel oops via invalid zone_device_data dereference on foreign DEVICE_COHERENT pages
--

commit 958d8ac307a125ba9f54f827cc57e2a2d00555d1
Author: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>

drm/xe: Bump prefetch BO LRU via GPUSVM devmem callback

This commit adds xe_svm_devmem_lru_bump() and sets it as the devmem_fn
callback in the prefetch worker to move backing BOs to the LRU tail
during get_pages.

> diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
> index f39e647512ad9..d07dd95ab4514 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
[ ... ]
> @@ -1625,6 +1626,39 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
>  	return err;
>  }
>  
> +/**
> + * xe_svm_devmem_lru_bump() - Move a range's backing BO to the TTM LRU tail
[ ... ]
> + */
> +void xe_svm_devmem_lru_bump(struct drm_pagemap_devmem *devmem_allocation)
> +{
> +#if IS_ENABLED(CONFIG_DRM_XE_PAGEMAP)
> +	struct xe_bo *bo;
> +
> +	if (devmem_allocation->ops != &dpagemap_devmem_ops)
> +		return;

[Severity: High]
Does this guard effectively protect against foreign allocations?

When a user maps a foreign DEVICE_COHERENT page (e.g., from an AMDGPU
allocation) and binds it into an xe VM, this guard appears to be reached
too late.

The kernel oops seems to occur earlier in the call chain when
drm_gpusvm_walk_devmem() blindly calls drm_pagemap_page_to_devmem(). 

That helper erroneously casts the foreign driver's zone_device_data to a
struct drm_pagemap_zdd pointer and dereferences zdd->devmem_allocation
before this ops check is ever executed.

> +
> +	bo = to_xe_bo(devmem_allocation);
> +
> +	if (!dma_resv_trylock(bo->ttm.base.resv))
> +		return;
> +
> +	ttm_bo_move_to_lru_tail_unlocked(&bo->ttm);
> +	dma_resv_unlock(bo->ttm.base.resv);
> +#endif
> +}
> +
[ ... ]
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 425c678f4480a..a42d86f674cc8 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -3242,6 +3242,7 @@ static int prefetch_ranges(struct xe_vm *vm, struct xe_vma_ops *vops,
>  	ctx.devmem_possible = devmem_possible;
>  	ctx.check_pages_threshold = devmem_possible ? SZ_64K : 0;
>  	ctx.device_private_page_owner = xe_svm_private_page_owner(vm, !dpagemap);
> +	ctx.devmem_fn = xe_svm_devmem_lru_bump;

[Severity: High]
Can setting this callback trigger a denial of service by unprivileged users?

Setting ctx.devmem_fn activates the drm_gpusvm_walk_devmem() code path
during hmm_range_fault() processing. 

If a user maps a foreign DEVICE_COHERENT page, hmm_range_fault() returns a
valid PFN, and the walker will process it, leading to the invalid
zone_device_data dereference mentioned above.

>  
>  	skip_threads =  op->prefetch_range.ranges_count == 1 ||
>  		(!dpagemap && !(vops->flags &

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001033602.3166274-6-himal.prasad.ghimiray@intel.com?part=3