vmw_gb_surface_reference_internal() checks srf->res.guest_memory_bo
for NULL and reads its vma_node offset and base.size outside the
cmdbuf_mutex scope that protects the backup pointer.
A concurrent backup switch, e.g. from SVGA_3D_CMD_BIND_GB_SURFACE
via vmw_resource_unreserve(), drops its reference to the old buffer
and installs a new one under cmdbuf_mutex. The unlocked check in
the surface reference ioctl can see a transient NULL. The reads
after the unlock can return another buffer's offset and size; a
further switch can drop that buffer's last reference and free it
while the ioctl reads from it.
Move the NULL check inside the mutex, and read buffer_map_handle
and buffer_size while still holding it.
Fixes: a97e21923b42 ("drm/vmwgfx: Hook up guest-backed surfaces")
Cc: [email protected]
Reported-by: Sashiko <[email protected]>
Signed-off-by: Aldo Ariel Panzardo <[email protected]>
---
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 97966d849583..291e3efad9ad 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -1718,23 +1718,32 @@ vmw_gb_surface_reference_internal(struct drm_device
*dev,
user_srf = container_of(base, struct vmw_user_surface, prime.base);
srf = &user_srf->srf;
+ metadata = &srf->metadata;
+
+ mutex_lock(&dev_priv->cmdbuf_mutex);
+
if (!srf->res.guest_memory_bo) {
+ mutex_unlock(&dev_priv->cmdbuf_mutex);
DRM_ERROR("Shared GB surface is missing a backup buffer.\n");
ret = -EINVAL;
goto out_bad_resource;
}
- metadata = &srf->metadata;
- mutex_lock(&dev_priv->cmdbuf_mutex); /* Protect res->backup */
ret = drm_gem_handle_create(file_priv,
&srf->res.guest_memory_bo->tbo.base,
&backup_handle);
- mutex_unlock(&dev_priv->cmdbuf_mutex);
if (ret != 0) {
+ mutex_unlock(&dev_priv->cmdbuf_mutex);
drm_err(dev, "Wasn't able to create a backing handle for
surface sid = %u.\n",
req->sid);
goto out_bad_resource;
}
+ rep->crep.buffer_map_handle =
+
drm_vma_node_offset_addr(&srf->res.guest_memory_bo->tbo.base.vma_node);
+ rep->crep.buffer_size = srf->res.guest_memory_bo->tbo.base.size;
+
+ mutex_unlock(&dev_priv->cmdbuf_mutex);
+
rep->creq.base.svga3d_flags = SVGA3D_FLAGS_LOWER_32(metadata->flags);
rep->creq.base.format = metadata->format;
rep->creq.base.mip_levels = metadata->mip_levels[0];
@@ -1747,9 +1756,6 @@ vmw_gb_surface_reference_internal(struct drm_device *dev,
rep->crep.handle = user_srf->prime.base.handle;
rep->crep.backup_size = srf->res.guest_memory_size;
rep->crep.buffer_handle = backup_handle;
- rep->crep.buffer_map_handle =
-
drm_vma_node_offset_addr(&srf->res.guest_memory_bo->tbo.base.vma_node);
- rep->crep.buffer_size = srf->res.guest_memory_bo->tbo.base.size;
rep->creq.version = drm_vmw_gb_surface_v1;
rep->creq.svga3d_flags_upper_32_bits =
--
2.43.0
