* [PATCH] drm/vc4: fix krealloc() memory leak [not found] <20260526184105.18962-1-grandmaster@al2klimov.de> @ 2026-05-26 18:41 ` Alexander A. Klimov 2026-05-27 4:21 ` Claude review: " Claude Code Review Bot 2026-05-27 4:21 ` Claude Code Review Bot 0 siblings, 2 replies; 3+ messages in thread From: Alexander A. Klimov @ 2026-05-26 18:41 UTC (permalink / raw) To: Maxime Ripard, Dave Stevenson, Maíra Canal, Raspberry Pi Kernel Maintenance, Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter, Eric Anholt, open list:DRM DRIVERS Cc: Alexander A. Klimov Don't just overwrite the original pointer passed to krealloc() with its return value without checking latter: MEM = krealloc(MEM, SZ, GFP); If krealloc() returns NULL, that erases the pointer to the still allocated memory, hence leaks this memory. Instead, use a temporary variable, check it's not NULL and only then assign it to the original pointer: TMP = krealloc(MEM, SZ, GFP); if (!TMP) return; MEM = TMP; Fixes: 6d45c81d229d ("drm/vc4: Add support for branching in shader validation.") Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de> --- drivers/gpu/drm/vc4/vc4_validate_shaders.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_validate_shaders.c b/drivers/gpu/drm/vc4/vc4_validate_shaders.c index d48cf76983c0..af48758592f9 100644 --- a/drivers/gpu/drm/vc4/vc4_validate_shaders.c +++ b/drivers/gpu/drm/vc4/vc4_validate_shaders.c @@ -291,14 +291,14 @@ static bool require_uniform_address_uniform(struct vc4_validated_shader_info *va uint32_t o = validated_shader->num_uniform_addr_offsets; uint32_t num_uniforms = validated_shader->uniforms_size / 4; - validated_shader->uniform_addr_offsets = - krealloc(validated_shader->uniform_addr_offsets, - (o + 1) * - sizeof(*validated_shader->uniform_addr_offsets), - GFP_KERNEL); - if (!validated_shader->uniform_addr_offsets) + void *mem = krealloc(validated_shader->uniform_addr_offsets, + (o + 1) * + sizeof(*validated_shader->uniform_addr_offsets), + GFP_KERNEL); + if (!mem) return false; + validated_shader->uniform_addr_offsets = mem; validated_shader->uniform_addr_offsets[o] = num_uniforms; validated_shader->num_uniform_addr_offsets++; -- 2.54.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Claude review: drm/vc4: fix krealloc() memory leak 2026-05-26 18:41 ` [PATCH] drm/vc4: fix krealloc() memory leak Alexander A. Klimov @ 2026-05-27 4:21 ` Claude Code Review Bot 2026-05-27 4:21 ` Claude Code Review Bot 1 sibling, 0 replies; 3+ messages in thread From: Claude Code Review Bot @ 2026-05-27 4:21 UTC (permalink / raw) To: dri-devel-reviews Overall Series Review Subject: drm/vc4: fix krealloc() memory leak Author: "Alexander A. Klimov" <grandmaster@al2klimov.de> Patches: 1 Reviewed: 2026-05-27T14:21:18.853622 --- This is a single patch fixing a genuine krealloc() memory leak bug in the VC4 shader validation code. The bug pattern is well-known: when `krealloc()` fails and returns NULL, writing that NULL back to the original pointer loses the reference to the still-allocated old memory. The fix is correct and minimal. The patch is part of a larger series (indicated by the `In-Reply-To` header referencing a `*-1-*` message), presumably fixing this same krealloc pattern across multiple drivers. **Verdict: The patch is correct and should be applied.** --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 3+ messages in thread
* Claude review: drm/vc4: fix krealloc() memory leak 2026-05-26 18:41 ` [PATCH] drm/vc4: fix krealloc() memory leak Alexander A. Klimov 2026-05-27 4:21 ` Claude review: " Claude Code Review Bot @ 2026-05-27 4:21 ` Claude Code Review Bot 1 sibling, 0 replies; 3+ messages in thread From: Claude Code Review Bot @ 2026-05-27 4:21 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Correctness: Good.** The fix correctly addresses a real memory leak. In the original code: ```c validated_shader->uniform_addr_offsets = krealloc(validated_shader->uniform_addr_offsets, (o + 1) * sizeof(*validated_shader->uniform_addr_offsets), GFP_KERNEL); if (!validated_shader->uniform_addr_offsets) return false; ``` If `krealloc()` returns NULL, the original allocation pointed to by `uniform_addr_offsets` is leaked because the pointer is overwritten before the NULL check. The caller chain eventually calls `kfree(validated_shader->uniform_addr_offsets)` in the cleanup paths (`vc4_validate_shaders.c:951`, `vc4_bo.c:176`, `vc4_bo.c:585`), but since `uniform_addr_offsets` is now NULL, the free is a no-op and the original memory leaks. **Fix approach:** The fix uses a `void *mem` temporary variable and only assigns to `validated_shader->uniform_addr_offsets` after confirming non-NULL: ```c void *mem = krealloc(validated_shader->uniform_addr_offsets, (o + 1) * sizeof(*validated_shader->uniform_addr_offsets), GFP_KERNEL); if (!mem) return false; validated_shader->uniform_addr_offsets = mem; ``` This preserves the original pointer on failure so the existing cleanup paths can properly free it. **Minor style observation:** The temporary is typed `void *` rather than `uint32_t *` (the actual type of `uniform_addr_offsets` as seen in `vc4_drv.h:858`). This works correctly since C allows implicit conversion from `void *`, and it's idiomatic for krealloc wrappers. However, using `uint32_t *mem` would be slightly more type-safe and avoid the implicit cast. This is a matter of taste and not a blocker — the series likely uses `void *` consistently across all patches for the same krealloc pattern. **Fixes tag:** Correctly references commit `6d45c81d229d ("drm/vc4: Add support for branching in shader validation.")` which introduced this code. **Scope:** The change is minimal and precisely scoped — only the 6 lines around the krealloc call are modified, with no functional change on the success path. **Recommendation: Patch is correct. Apply as-is.** --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-27 4:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260526184105.18962-1-grandmaster@al2klimov.de>
2026-05-26 18:41 ` [PATCH] drm/vc4: fix krealloc() memory leak Alexander A. Klimov
2026-05-27 4:21 ` Claude review: " Claude Code Review Bot
2026-05-27 4:21 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox