Re: [PATCH 2/4] amdgpu: fix a race in kfd_mem_export_dmabuf()

[Felix Kuehling <felix.kuehling@amd.com>]

On 2024-08-12 02:59, Al Viro wrote:
> Using drm_gem_prime_handle_to_fd() to set dmabuf up and insert it into
> descriptor table, only to have it looked up by file descriptor and
> remove it from descriptor table is not just too convoluted - it's
> racy; another thread might have modified the descriptor table while
> we'd been going through that song and dance.
>
> Switch kfd_mem_export_dmabuf() to using drm_gem_prime_handle_to_dmabuf()
> and leave the descriptor table alone...
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

This patch is

Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 11672bfe4fad..bc5401de2948 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -25,7 +25,6 @@
>   #include <linux/pagemap.h>
>   #include <linux/sched/mm.h>
>   #include <linux/sched/task.h>
> -#include <linux/fdtable.h>
>   #include <drm/ttm/ttm_tt.h>
>   
>   #include <drm/drm_exec.h>
> @@ -818,18 +817,13 @@ static int kfd_mem_export_dmabuf(struct kgd_mem *mem)
>   	if (!mem->dmabuf) {
>   		struct amdgpu_device *bo_adev;
>   		struct dma_buf *dmabuf;
> -		int r, fd;
>   
>   		bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
> -		r = drm_gem_prime_handle_to_fd(&bo_adev->ddev, bo_adev->kfd.client.file,
> +		dmabuf = drm_gem_prime_handle_to_dmabuf(&bo_adev->ddev, bo_adev->kfd.client.file,
>   					       mem->gem_handle,
>   			mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ?
> -					       DRM_RDWR : 0, &fd);
> -		if (r)
> -			return r;
> -		dmabuf = dma_buf_get(fd);
> -		close_fd(fd);
> -		if (WARN_ON_ONCE(IS_ERR(dmabuf)))
> +					       DRM_RDWR : 0);
> +		if (IS_ERR(dmabuf))
>   			return PTR_ERR(dmabuf);
>   		mem->dmabuf = dmabuf;
>   	}