Re: [PATCH 1/3] drm/xe: don't allocate fake mmap offsets for imported dma-bufs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/xe: bogus offset 0 in drm_gem_ttm_dumb_map_offset() for
  imported BOs
--

commit f6897baa0037e7c5c3efac24443f4046494f642e
Author: Matthew Auld <matthew.auld@intel.com>

drm/xe: don't allocate fake mmap offsets for imported dma-bufs

This commit removes the drm_vma_offset node from imported dma-buf buffer
objects after initialization and rejects objects without a valid vma_node
in xe_gem_mmap_offset_ioctl().

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 42a6aa1a2e4a0..0181f73ef66d0 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -3851,6 +3851,12 @@ int xe_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
>  	if (XE_IOCTL_DBG(xe, !gem_obj))
>  		return -ENOENT;
>  
> +	/* Imported dma-bufs have no mmap offset since CPU access is rejected. */
> +	if (XE_IOCTL_DBG(xe, !drm_vma_node_size(&gem_obj->vma_node))) {
> +		xe_bo_put(gem_to_xe_bo(gem_obj));
> +		return -EINVAL;
> +	}
> +
>  	/* The mmap offset was set up at BO allocation time. */
>  	args->offset = drm_vma_node_offset_addr(&gem_obj->vma_node);
>  
> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c
> index 5bf13dc4b61d8..17a8663a68de0 100644
> --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> @@ -12,6 +12,7 @@
>  
>  #include <drm/drm_device.h>
>  #include <drm/drm_prime.h>
> +#include <drm/drm_vma_manager.h>
>  #include <drm/ttm/ttm_tt.h>
>  
>  #include "tests/xe_test.h"
> @@ -353,6 +354,14 @@ xe_dma_buf_create_obj(struct drm_device *dev, struct dma_buf *dma_buf)
>  	}
>  	drm_gem_object_put(dummy_obj);
>  
> +	/*
> +	 * CPU faults on imported BOs are rejected, so there is no point in
> +	 * consuming fake mmap offset space for them.
> +	 */
> +	if (!ret)
> +		drm_vma_offset_remove(bo->ttm.bdev->vma_manager,
> +				      &bo->ttm.base.vma_node);
> +
>  	return ret ? ERR_PTR(ret) : &bo->ttm.base;
>  }

[Severity: Medium]
Does removing vma_node here cause DRM_IOCTL_MODE_MAP_DUMB on an imported
dma-buf handle to succeed with a bogus offset of 0?

regular_driver sets .dumb_map_offset = drm_gem_ttm_dumb_map_offset, which
does not check drm_gem_is_imported() or drm_vma_node_size() before reading
the offset:

drivers/gpu/drm/drm_gem_ttm_helper.c:drm_gem_ttm_dumb_map_offset() {
	...
	*offset = drm_vma_node_offset_addr(&gem->vma_node);

	drm_gem_object_put(gem);

	return 0;
}

With bo->ttm.base.vma_node zeroed by drm_vma_offset_remove(),
drm_gem_ttm_dumb_map_offset() returns 0 with *offset = 0 instead of
-EINVAL.

Also, because xe_bo_init_locked() -> ttm_bo_init_reserved() still
transiently allocates the vma_node for ttm_bo_type_sg before
xe_dma_buf_create_obj() removes it, importing a dma-buf can still fail
with -ENOSPC when bdev->vma_manager is already full or fragmented.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107-oom-fun-v1-0-be9eb05b82ed@intel.com?part=1