[PATCH] drm/vmwgfx: Don't overwrite KMS surface dirty tracker

[PATCH] drm/vmwgfx: Don't overwrite KMS surface dirty tracker

[Ian Forbes]

We were overwriting the surface's dirty tracker here causing a memory leak.

Reported-by: Mika Penttilä <mpenttil@redhat.com>
Closes: https://lore.kernel.org/dri-devel/8c53f3c6-c6de-46fe-a8ca-d98dd52b3abe@redhat.com/
Fixes: 965544150d1c ("drm/vmwgfx: Refactor cursor handling")
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 55730e29d3ae..e7bddf840a79 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -771,7 +771,8 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct drm_device *dev,
 		ret = vmw_bo_dirty_add(bo);
 		if (!ret && surface && surface->res.func->dirty_alloc) {
 			surface->res.coherent = true;
-			ret = surface->res.func->dirty_alloc(&surface->res);
+			if (surface->res.dirty == NULL)
+				ret = surface->res.func->dirty_alloc(&surface->res);
 		}
 		ttm_bo_unreserve(&bo->tbo);
 	}
-- 
2.53.0

Claude review: drm/vmwgfx: Don't overwrite KMS surface dirty tracker

[Claude Code Review Bot]

Patch Review

**Correctness: Good.** The fix is sound. Looking at `vmw_surface_dirty_alloc()` at `vmwgfx_surface.c:2040`:

```c
res->dirty = (struct vmw_resource_dirty *) dirty;
```

This unconditionally overwrites `res->dirty`, so if a dirty tracker was already allocated, the previous allocation is leaked. The guard:

```c
if (surface->res.dirty == NULL)
    ret = surface->res.func->dirty_alloc(&surface->res);
```

correctly prevents the double allocation. This pattern is consistent with the other call site in `vmwgfx_resource.c:402` which also checks `!res->dirty` before calling `dirty_alloc`.

**One minor observation:** The existing code at line 772 already checks `surface->res.func->dirty_alloc` (that the function pointer is non-NULL) before entering the block. It might be slightly cleaner to also set `surface->res.coherent = true` only when the dirty tracker is actually being newly allocated (i.e., move it inside the `if (surface->res.dirty == NULL)` block), since if `dirty` is already allocated, coherent should presumably already be true. However, setting `coherent = true` redundantly is harmless — it's just a boolean assignment — so this is a style nit rather than a bug.

**Commit message and tags: Good.** Has `Reported-by`, `Closes` with lore link, `Fixes` tag referencing the commit that introduced the regression, and proper `Signed-off-by`. All in order.

**Verdict:** The patch is a clean, minimal fix for the reported memory leak. It looks good to merge.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vmwgfx: Don't overwrite KMS surface dirty tracker

[Claude Code Review Bot]

Overall Series Review

Subject: drm/vmwgfx: Don't overwrite KMS surface dirty tracker
Author: Ian Forbes <ian.forbes@broadcom.com>
Patches: 1
Reviewed: 2026-03-03T12:53:48.222615

---

This is a single-patch fix for a memory leak in the vmwgfx KMS framebuffer creation path. The fix is correct and well-targeted. When `vmw_kms_fb_create` is called for a surface that already has a dirty tracker allocated (i.e., `surface->res.dirty != NULL`), the code was unconditionally calling `dirty_alloc()`, which allocates a new dirty tracker and assigns it to `res->dirty` (see `vmwgfx_surface.c:2040`), overwriting the old pointer and leaking that memory. The fix simply guards the allocation with a NULL check, which is the right approach.

---
Generated by Claude Code Patch Reviewer