Re: [PATCH v2 03/11] drm/amdgpu/gem: only enforce amdgpu_bo access checks on amdgpu_bo objects

["Christian König" <christian.koenig@amd.com>]

On 4/27/26 10:34, Jesse Zhang wrote:
> From: "Jesse.zhang" <Jesse.zhang@amd.com>
> 
> amdgpu_mode_dumb_mmap() unconditionally cast every looked-up
> drm_gem_object to amdgpu_bo via gem_to_amdgpu_bo() and then read
> robj->tbo.ttm and robj->flags.  For a bare drm_gem_object created via
> drm_gem_private_object_init() with its own drm_gem_object_funcs (e.g.
> the SDMA UMQ doorbell pool BO),

Well big NAK to that approach.

Why in the world would we want to create a GEM object directly through drm_gem_private_object_init()?

Regards,
Christian.


> the cast yields a pointer to unrelated
> memory.  Whether that memory happens to look like a usermm or carry
> AMDGPU_GEM_CREATE_NO_CPU_ACCESS set is a function of allocator state,
> producing intermittent -EPERM returns from DRM_IOCTL_AMDGPU_GEM_MMAP.


> 
> Gate the amdgpu_bo-specific check on gobj->funcs matching
> amdgpu_gem_object_funcs, and resolve the mmap offset via the GEM
> vma_node directly so the path works for any drm_gem_object backed by
> this ioctl.
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 0071d6957828..ccb92088172c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -587,13 +587,22 @@ int amdgpu_mode_dumb_mmap(struct drm_file *filp,
>  	if (!gobj)
>  		return -ENOENT;
>  
> -	robj = gem_to_amdgpu_bo(gobj);
> -	if (amdgpu_ttm_tt_get_usermm(robj->tbo.ttm) ||
> -	    (robj->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
> -		drm_gem_object_put(gobj);
> -		return -EPERM;
> +	/*
> +	 * The amdgpu_bo-specific access checks below assume gobj is wrapped
> +	 * in an amdgpu_bo. Bare drm_gem_object instances (e.g., the SDMA UMQ
> +	 * doorbell pool BO created via drm_gem_private_object_init with its
> +	 * own funcs) are not amdgpu_bo, so gem_to_amdgpu_bo would dereference
> +	 * unrelated memory and intermittently return -EPERM.
> +	 */
> +	if (gobj->funcs == &amdgpu_gem_object_funcs) {
> +		robj = gem_to_amdgpu_bo(gobj);
> +		if (amdgpu_ttm_tt_get_usermm(robj->tbo.ttm) ||
> +		    (robj->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
> +			drm_gem_object_put(gobj);
> +			return -EPERM;
> +		}
>  	}
> -	*offset_p = amdgpu_bo_mmap_offset(robj);
> +	*offset_p = drm_vma_node_offset_addr(&gobj->vma_node);
>  	drm_gem_object_put(gobj);
>  	return 0;
>  }