Re: [PATCH v3 4/5] drm/vmwgfx: Change ttm refs for dumb buffers.

[Zack Rusin <zack.rusin@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 2999 bytes --]

On Thu, May 14, 2026 at 6:46 PM Maaz Mombasawala
<maaz.mombasawala@broadcom.com> wrote:
>
> Preserve a ttm reference during dumb buffer creation. This keeps the dumb
> buffer valid for framebuffer usage and fixes all igt tests that use dumb
> buffers.
> Also fix ttm_prime_fd_to_handle(), which in the error case was leaking a
> dma_buf reference. During vmw_prime_fd_to_handle() this function
> is expected to fail for dumb buffers since the fd is for a gem object,
> the dma_buf would in turn hold a reference to the dumb buffer gem object
> and cause a memory leak.
>
> Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/ttm_object.c     | 7 +++++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 5 +++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.c b/drivers/gpu/drm/vmwgfx/ttm_object.c
> index 2421b0dd057c..f9042bafdc93 100644
> --- a/drivers/gpu/drm/vmwgfx/ttm_object.c
> +++ b/drivers/gpu/drm/vmwgfx/ttm_object.c
> @@ -547,14 +547,17 @@ int ttm_prime_fd_to_handle(struct ttm_object_file *tfile,
>         if (IS_ERR(dma_buf))
>                 return PTR_ERR(dma_buf);
>
> -       if (dma_buf->ops != &tdev->ops)
> -               return -ENOSYS;
> +       if (dma_buf->ops != &tdev->ops) {
> +               ret = -ENOSYS;
> +               goto out;
> +       }
>
>         prime = (struct ttm_prime_object *) dma_buf->priv;
>         base = &prime->base;
>         *handle = base->handle;
>         ret = ttm_ref_object_add(tfile, base, NULL, false);
>
> +out:
>         dma_buf_put(dma_buf);
>
>         return ret;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index b2d3927b5567..256e7b89daea 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -2328,11 +2328,12 @@ int vmw_dumb_create(struct drm_file *file_priv,
>         struct vmw_user_surface *usurf = container_of(vbo->dumb_surface,
>                                                 struct vmw_user_surface, srf);
>         usurf->prime.base.refcount_release = NULL;
> +
>  err:
>         if (res)
>                 vmw_resource_unreference(&res);
> -
> -       ttm_ref_object_base_unref(tfile, arg.rep.handle);
> +       if (ret)
> +               ttm_ref_object_base_unref(tfile, arg.rep.handle);

That still doesn't look correct. dumb buffers own the resource and
should drop it themselves. What you did is made arg.rep.handle stay in
tdev->idr and now both the tfile and dumb_buffer own the surface.
Whichever one is destroyed first will make the other UAF on that
surface.

For all the commits in that series that are fixes please find a proper
Fixes commit. And if you want to land this through drm-misc-fixes then
you'll need to remove the last one from the series because I don't
think we should land general cleanups through fixes.

z

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]