On 18/02/2026 22:01, Melissa Wen wrote:
On 05/02/2026 18:31, Maíra Canal wrote:
When vc4_save_hang_state() encounters an early return condition, it
returns without freeing the previously allocated `kernel_state`,
leaking memory.
Add the missing kfree() calls in both early return paths.
Fixes: 214613656b51 ("drm/vc4: Add an interface for capturing the GPU
state after a hang.")
Signed-off-by: Maíra Canal <[email protected]>
---
drivers/gpu/drm/vc4/vc4_gem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/
vc4_gem.c
index
ab16164b5edaf382b9c4bf1f08766748cac77fcc..f943ff7da28ae528c0fdfac76e989a2d5286d193 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -172,6 +172,7 @@ vc4_save_hang_state(struct drm_device *dev)
exec[1] = vc4_first_render_job(vc4);
if (!exec[0] && !exec[1]) {
spin_unlock_irqrestore(&vc4->job_lock, irqflags);
+ kfree(kernel_state);
LGTM, but even better if you can centralize error handling in
vc4_free_hang_state() and point all returns to a single place.
More reviewers more opinions, so you have to live with the pros and cons
of that. :)
I chiming in with an alternative and that is to not even allocate
kernel_state before the "!exec[0] && !exec[1]" check. Or you could even
go further than that and allocate a single blob if struct vc4_hang_state
instead of pointer to array would contain a flexible built in array
(kmalloc_flex). Then there would be no need for any cleanup.
But granted, as this is a Fixes: patch it makes more sense to have it as
simple as possible for easy backporting and follow up with my suggestion
on top later, if you wish so.
Regards,
Tvrtko
return;
}
@@ -192,6 +193,7 @@ vc4_save_hang_state(struct drm_device *dev)
if (!kernel_state->bo) {
spin_unlock_irqrestore(&vc4->job_lock, irqflags);
+ kfree(kernel_state);
return;
}
