[PATCH] drm/vmwgfx: Add error path for xa_store in vmw_bo_add_detached_resource

[Zack Rusin]

On Tue, Feb 25, 2025 at 9:54 AM Keisuke Nishimura
<keisuke.nishimura at inria.fr> wrote:
>
> The xa_store() may fail due to memory allocation failure because there
> is no guarantee that the index is already used. This fix introduces new
> paths to handle the error.
>
> This patch also aligns the order of function calls by calling
> vmw_bo_add_detached_resource() before ttm_prime_object_init() in order
> to allow consistent error handling.
>
> Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
> Signed-off-by: Keisuke Nishimura <keisuke.nishimura at inria.fr>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c      |  4 ++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.h      |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 16 ++++++++++++++--
>  3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 9b5b8c1f063b..aa13e4061ff1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -848,9 +848,9 @@ void vmw_bo_placement_set_default_accelerated(struct vmw_bo *bo)
>         vmw_bo_placement_set(bo, domain, domain);
>  }
>
> -void vmw_bo_add_detached_resource(struct vmw_bo *vbo, struct vmw_resource *res)
> +int vmw_bo_add_detached_resource(struct vmw_bo *vbo, struct vmw_resource *res)
>  {
> -       xa_store(&vbo->detached_resources, (unsigned long)res, res, GFP_KERNEL);
> +       return xa_err(xa_store(&vbo->detached_resources, (unsigned long)res, res, GFP_KERNEL));
>  }
>
>  void vmw_bo_del_detached_resource(struct vmw_bo *vbo, struct vmw_resource *res)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> index 11e330c7c7f5..51790a11fe64 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> @@ -141,7 +141,7 @@ void vmw_bo_move_notify(struct ttm_buffer_object *bo,
>                         struct ttm_resource *mem);
>  void vmw_bo_swap_notify(struct ttm_buffer_object *bo);
>
> -void vmw_bo_add_detached_resource(struct vmw_bo *vbo, struct vmw_resource *res);
> +int vmw_bo_add_detached_resource(struct vmw_bo *vbo, struct vmw_resource *res);
>  void vmw_bo_del_detached_resource(struct vmw_bo *vbo, struct vmw_resource *res);
>  struct vmw_surface *vmw_bo_surface(struct vmw_bo *vbo);
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index 5721c74da3e0..1f7626f6ac0b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -871,7 +871,12 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data,
>                         vmw_resource_unreference(&res);
>                         goto out_unlock;
>                 }
> -               vmw_bo_add_detached_resource(res->guest_memory_bo, res);
> +
> +               ret = vmw_bo_add_detached_resource(res->guest_memory_bo, res);
> +               if (unlikely(ret != 0)) {
> +                       vmw_resource_unreference(&res);
> +                       goto out_unlock;
> +               }
>         }
>
>         tmp = vmw_resource_reference(&srf->res);
> @@ -1670,6 +1675,14 @@ vmw_gb_surface_define_internal(struct drm_device *dev,
>
>         }
>
> +       if (res->guest_memory_bo) {
> +               ret = vmw_bo_add_detached_resource(res->guest_memory_bo, res);
> +               if (unlikely(ret != 0)) {
> +                       vmw_resource_unreference(&res);
> +                       goto out_unlock;
> +               }
> +       }
> +
>         tmp = vmw_resource_reference(res);
>         ret = ttm_prime_object_init(tfile, res->guest_memory_size, &user_srf->prime,
>                                     VMW_RES_SURFACE,
> @@ -1684,7 +1697,6 @@ vmw_gb_surface_define_internal(struct drm_device *dev,
>         rep->handle      = user_srf->prime.base.handle;
>         rep->backup_size = res->guest_memory_size;
>         if (res->guest_memory_bo) {
> -               vmw_bo_add_detached_resource(res->guest_memory_bo, res);
>                 rep->buffer_map_handle =
>                         drm_vma_node_offset_addr(&res->guest_memory_bo->tbo.base.vma_node);
>                 rep->buffer_size = res->guest_memory_bo->tbo.base.size;
> --
> 2.34.1
>

Looks good. Thank you for the patch! I'll push it to drm-misc-next for
now because I'd like it alongside the cursor rework.

Reviewed-by: Zack Rusin <zack.rusin at broadcom.com>

z
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5427 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20250304/22b675f5/attachment.bin>