* [PATCH] drm/amdgpu: fix lock leak on ENOMEM in AMDGPU_GEM_OP_GET_MAPPING_INFO
@ 2026-05-17 13:17 Michael Bommarito
2026-05-18 6:08 ` Claude review: " Claude Code Review Bot
2026-05-18 6:08 ` Claude Code Review Bot
0 siblings, 2 replies; 3+ messages in thread
From: Michael Bommarito @ 2026-05-17 13:17 UTC (permalink / raw)
To: Alex Deucher, Christian Koenig, David Francis, Sumit Semwal,
David Airlie, Simona Vetter, amd-gfx, dri-devel, linux-kernel,
linux-media, linaro-mm-sig
Cc: Ziyi Guo
The AMDGPU_GEM_OP_GET_MAPPING_INFO branch of amdgpu_gem_op_ioctl()
holds three cleanup-tracked resources before calling kvcalloc():
the drm_gem_object reference from drm_gem_object_lookup(), the
drm_exec lock on the looked-up GEM via drm_exec_lock_obj(), and
the drm_exec lock on the per-process VM root page directory via
amdgpu_vm_lock_pd(). All three are released by the out_exec
label that every other error path in this function jumps to.
The kvcalloc() failure path returns -ENOMEM directly, skipping
out_exec and leaking all three.
The leaked per-process VM root PD dma_resv lock is the
load-bearing leak: any subsequent operation on the same VM
(further GEM ops, command-submission, eviction, TTM shrinker
callbacks) blocks on the held lock. DRM_IOCTL_AMDGPU_GEM_OP is
DRM_AUTH | DRM_RENDER_ALLOW, so this is an unprivileged-local
denial of service against the caller's GPU context, reachable
by any process with /dev/dri/renderD* access.
Route the failure through out_exec so drm_exec_fini() and
drm_gem_object_put() run.
Reproduced on stock 7.0.0-10, Ryzen 7 5700U / Radeon Vega
(Lucienne): the failing ioctl returns -ENOMEM and a second
GET_MAPPING_INFO on the same fd then blocks in
drm_exec_lock_obj() on the leaked dma_resv. SIGKILL on the
caller does not reap the task; the fd-release path during
process exit goes through amdgpu_gem_object_close() ->
drm_exec_prepare_obj() on the same lock, leaving the task in D
state until the box is rebooted. The patched kernel was not
rebuilt and re-tested on this hardware; the fix is mechanical.
Tested on a single Lucienne / Vega box only.
Ziyi Guo posted an independent INT_MAX-bound check for
args->num_entries in the same branch [1]; the two patches are
complementary and can land in either order.
Fixes: 4d82724f7f2b ("drm/amdgpu: Add mapping info option for GEM_OP ioctl")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/20260208000255.4073363-1-n7l8m4@u.northwestern.edu/ # [1]
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ef80bca4102..8224fb499fdf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -1091,8 +1091,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
* be retried.
*/
vm_entries = kvcalloc(args->num_entries, sizeof(*vm_entries), GFP_KERNEL);
- if (!vm_entries)
- return -ENOMEM;
+ if (!vm_entries) {
+ r = -ENOMEM;
+ goto out_exec;
+ }
amdgpu_vm_bo_va_for_each_valid_mapping(bo_va, mapping) {
if (num_mappings < args->num_entries) {
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Claude review: drm/amdgpu: fix lock leak on ENOMEM in AMDGPU_GEM_OP_GET_MAPPING_INFO
2026-05-17 13:17 [PATCH] drm/amdgpu: fix lock leak on ENOMEM in AMDGPU_GEM_OP_GET_MAPPING_INFO Michael Bommarito
@ 2026-05-18 6:08 ` Claude Code Review Bot
2026-05-18 6:08 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-18 6:08 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/amdgpu: fix lock leak on ENOMEM in AMDGPU_GEM_OP_GET_MAPPING_INFO
Author: Michael Bommarito <michael.bommarito@gmail.com>
Patches: 1
Reviewed: 2026-05-18T16:08:13.978576
---
This is a single-patch series fixing a genuine resource leak bug in `amdgpu_gem_op_ioctl()`. The analysis is correct: when `kvcalloc()` fails at line 1093, the original code returned `-ENOMEM` directly, bypassing the `out_exec` cleanup label. This leaked the `drm_exec` lock context (which holds the GEM object's `dma_resv` and the VM root PD's `dma_resv`) and the `drm_gem_object` reference.
The fix is mechanical and obviously correct — it sets `r = -ENOMEM` and jumps to `out_exec`, which calls `drm_exec_fini(&exec)` and `drm_gem_object_put(gobj)`, matching the cleanup pattern used by every other error path in this function.
The commit message is thorough and well-written. It identifies the specific resources leaked, explains the user-visible impact (unprivileged local DoS via stuck D-state task), references the related work by Ziyi Guo on `num_entries` validation, and correctly notes the fix was not runtime-tested on patched kernel. The `Fixes:` and `Cc: stable` tags are appropriate.
**Verdict: This patch looks correct and ready to apply.**
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
* Claude review: drm/amdgpu: fix lock leak on ENOMEM in AMDGPU_GEM_OP_GET_MAPPING_INFO
2026-05-17 13:17 [PATCH] drm/amdgpu: fix lock leak on ENOMEM in AMDGPU_GEM_OP_GET_MAPPING_INFO Michael Bommarito
2026-05-18 6:08 ` Claude review: " Claude Code Review Bot
@ 2026-05-18 6:08 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-18 6:08 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness: Good.** Looking at the function structure in `amdgpu_gem.c:1002-1141`:
1. At line 1016, `drm_gem_object_lookup()` acquires a reference on `gobj`.
2. At lines 1022-1036, `drm_exec_init()` + `drm_exec_lock_obj()` + `amdgpu_vm_lock_pd()` acquire locks tracked by `exec`.
3. The `out_exec` label at line 1137-1140 properly cleans up both via `drm_exec_fini(&exec)` and `drm_gem_object_put(gobj)`.
The original code at the `kvcalloc` failure:
```c
if (!vm_entries)
return -ENOMEM;
```
skips both cleanup calls. The fix:
```c
if (!vm_entries) {
r = -ENOMEM;
goto out_exec;
}
```
correctly routes through the existing cleanup path.
**Comparison with sibling error paths:** Every other error path in this function either jumps to `out_exec` (lines 1028, 1033, 1055-1056, 1059-1060, 1065-1066) or calls `drm_exec_fini(&exec)` inline before the `break` (lines 1047, 1079, 1119, 1131). The fix is consistent with the `goto out_exec` pattern used for all pre-`break` errors.
**One minor observation (not blocking):** The commit message mentions the patch was `Assisted-by: Claude:claude-opus-4-7`. This tag is not a standard kernel tag — the kernel community hasn't settled on a convention for AI-assisted patches. Some maintainers may request removal or conversion. This is a social/process concern, not a code correctness issue.
**No issues found with the code change itself.** The fix is minimal, correct, and addresses a real unprivileged local DoS.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-18 6:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-17 13:17 [PATCH] drm/amdgpu: fix lock leak on ENOMEM in AMDGPU_GEM_OP_GET_MAPPING_INFO Michael Bommarito
2026-05-18 6:08 ` Claude review: " Claude Code Review Bot
2026-05-18 6:08 ` 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