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
