[PATCH v2] drm/vmwgfx: return an error when a shared GB surface has no backup buffer

vmw_gb_surface_reference_internal() rejects a surface that has no backup
buffer by logging an error and jumping to out_bad_resource, but it never
sets ret on that path. ret is still 0 from the earlier successful
vmw_surface_handle_reference(), so the ioctl returns success while
leaving the reply structure uninitialised. drm_ioctl() then copies the
72-byte reply back to user space; on a CONFIG_INIT_STACK_NONE build that
is an information leak of stale stack contents, and on every build user
space is handed a "successful" reply that was never produced.
DRM_VMW_GB_SURFACE_REF is DRM_RENDER_ALLOW, so an unprivileged local
user can reach it.

Set ret to -EINVAL and, on the error paths, drop the reference that
vmw_surface_handle_reference() took; once the ioctl fails user space no
longer releases it via DRM_VMW_UNREF_SURFACE. The unref is guarded by
ret so the shared success fall-through keeps the reference the caller
still owns, and it also plugs the same leak on the drm_gem_handle_create()
failure path.

Fixes: 14b1c33e8429 ("drm/vmwgfx: Add new ioctl for GB surface create and reference")
Cc: [email protected]
Suggested-by: Zack Rusin <[email protected]>
Signed-off-by: Aldo Ariel Panzardo <[email protected]>
---
v2:
 - Shortened the commit message (Zack Rusin).
 - Also drop the reference taken by vmw_surface_handle_reference() on the
   error paths; with the ioctl now returning an error, user space no
   longer releases it via DRM_VMW_UNREF_SURFACE (Zack Rusin). This also
   plugs the same pre-existing leak on the drm_gem_handle_create() path.

v1: https://lore.kernel.org/all/[email protected]/

 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index bd0563741e..164886a3e6 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -1720,6 +1720,7 @@ vmw_gb_surface_reference_internal(struct drm_device *dev,
 	srf = &user_srf->srf;
 	if (!srf->res.guest_memory_bo) {
 		DRM_ERROR("Shared GB surface is missing a backup buffer.\n");
+		ret = -EINVAL;
 		goto out_bad_resource;
 	}
 	metadata = &srf->metadata;
@@ -1758,6 +1759,9 @@ vmw_gb_surface_reference_internal(struct drm_device *dev,
 	rep->creq.must_be_zero = 0;
 
 out_bad_resource:
+	if (ret)
+		ttm_ref_object_base_unref(vmw_fpriv(file_priv)->tfile,
+					  base->handle);
 	ttm_base_object_unref(&base);
 
 	return ret;
-- 
2.43.0