[PATCH] drm/amdkfd: don't leak BOs when process teardown can't unmap them

[PATCH] drm/amdkfd: don't leak BOs when process teardown can't unmap them

[Bocaj Gnuoy]

kfd_process_device_free_bos() discards the return value of both
amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu() and
amdgpu_amdkfd_gpuvm_free_memory_of_gpu(), then drops the idr handle
unconditionally.  That is fine as long as the free succeeds, but the unmap
has to validate the page-table BOs first (vm_validate_pt_pd_bos()), and
that allocates.  When the process is dying *because* memory is exhausted -
e.g. GTT pinned up to amdgpu.gttsize by a compute client that then aborted
on ENOMEM - the unmap fails with "failed to validate PT BOs",
mem->mapped_to_gpu_memory stays non-zero, and
amdgpu_amdkfd_gpuvm_free_memory_of_gpu() bails out with -EBUSY before it
removes the BO from process_info->kfd_bo_list.  The handle is dropped
anyway, so nothing will ever free that kgd_mem again.

The consequences are worse than a leak.  drm_release() ->
amdgpu_vm_fini() -> amdgpu_amdkfd_gpuvm_destroy_cb() then trips

  WARN_ON(!list_empty(&process_info->kfd_bo_list));
  WARN_ON(!list_empty(&process_info->userptr_inval_list));

and frees process_info regardless, while the leaked BO stays in TTM's
eviction LRU with bo_vas pointing into the just-freed amdgpu_vm.  A
subsequent amdgpu_gem_create_ioctl() that forces TTM eviction can then walk
into it and spin in amdgpu_vm_bo_move() -> _raw_spin_lock(), which produces
repeating rcu_preempt self-detected stalls, blocks reclaim-related work
(__lru_add_drain_all()) and eventually makes the machine unusable - within
about ten minutes here.  A recoverable ENOMEM becomes an unrecoverable
hang.

Note the free path itself does not allocate - it reserves the BO, detaches
the attachments and drops the references - so it can complete even when the
unmap could not.  Give amdgpu_amdkfd_gpuvm_free_memory_of_gpu() a @force
flag and set it on the teardown callers, which have no later chance to try
again.  The -EBUSY guard is kept for the ioctl paths, where userspace can
still unmap and retry, and where kfd_ioctl_free_memory_of_gpu() already
leaves the handle in place on failure.

Reported on: RX 6900 XT (gfx1030), amdgpu 3.64.0, kernel 7.1.6/7.1.8,
booted with amdgpu.gttsize=4096.  The failing allocation was on the Navi 21
at 0000:06:00.0, which enumerates as ROCm0; a gfx1100 is also present in
the same box but was not the device that ran out.

Tooling disclosure, per Documentation/process/generated-content.rst: this
bug was diagnosed and this patch written with the assistance of Claude Code
(claude-opus-5) in a single interactive session. The input was a kernel
trace captured on the reporting host - two WARNs out of
amdgpu_amdkfd_gpuvm_destroy_cb() during an aborting client's teardown,
followed by an unrecoverable deadlock in the TTM eviction path in the next
process to allocate - together with the request to find the cause and, if
possible, fix it. The assistant read the KFD BO lifecycle in the v7.1.8 and
v7.2 sources, identified the -EBUSY early return as the point at which the
BO is stranded on the process_info lists, and wrote both the diff and this
changelog. The diff was then rebased onto amd-staging-drm-next and the
call-site audit redone against that tree: one prototype, one definition and
eight call sites, all converted. The rebase caught a real miss - the tree
has gained kfd_process_free_gpuvm_map() alongside kfd_process_free_gpuvm(),
and the original hunk's trailing context matched the wrong one of the two.
Both are teardown-only (kfd_process_destroy_pdds() and the create_process()
error unwind), so both take force=true. checkpatch.pl was the only
additional analysis tool used.

Tested on a RX 6900 XT + RX 7900 XTX box, kernel 7.2.0-1-cachyos, booted
with amdgpu.gttsize=4096 so the GTT wall is reachable. A HIP-linked client
(llama.cpp with both the HIP and Vulkan backends present) was made to pin
host memory up to the 4 GiB cap and die on the resulting ENOMEM. Same
kernel version, same cap, same workload, same binary either side; the
patch is the only variable:

                              unpatched   patched
  peak GTT                      4.00 GiB   4.00 GiB
  "failed to validate PT BOs"         34         36
  exit                           139/SEGV   139/SEGV
  teardown WARNs                       2          0
  "Force-freeing BO VA"                0          9

Both sides died the same way, on SIGSEGV, so the difference is in teardown
and not in how the client failed.  The force-free count of 9 matched the 9
KFD allocations an earlier kprobe run measured for this client, consistent
with all of those allocations being reclaimed.

End to end: on the unpatched kernel, relaunching the same workload after
that abort is what hangs the machine - the new client's
amdgpu_gem_create_ioctl walks the eviction LRU into the orphaned object.
With the patch, a second run under heavier pressure (1904 "failed to
validate PT BOs", 35 force-freed BOs) again left zero WARNs, and the
relaunch then loaded in 12 seconds and served normally instead of
deadlocking. GTT returned to its 0.05 GiB idle baseline afterwards, so the
memory is genuinely reclaimed rather than merely not deadlocking. Across
four runs the client died variously on SIGABRT and SIGSEGV; the leak
tracked memory exhaustion at teardown, never the signal.

Note the WARNs that fire vary between runs: the originally reported crash
tripped :1608 and :1610 (kfd_bo_list, userptr_inval_list), while this
reproduction tripped :1608 and :1609 (kfd_bo_list, userptr_valid_list).
Both are the same skipped list_del_init(&mem->validate_list) - that node
serves whichever list the BO currently sits on, so the leak is
list-agnostic. The client also died on SIGSEGV rather than the abort seen
originally; the leak depends on memory being exhausted at teardown, not on
how the process died, which is consistent with SIGKILL alone never
reproducing it.

Limitations, per Documentation/process/coding-assistants.rst step 8:
- The deadlock itself was reproduced on the unpatched kernel, but its stack
  trace was not captured: the jammed workqueue takes journald down with it,
  so nothing reaches disk. Capturing it needs netconsole or a serial
  console. The teardown WARNs, which are the direct measure of the leak,
  are captured in full.
- The claim that the deadlocked spinlock lives inside the leaked object
  still rests on address arithmetic (lock at the WARNed structure + 0x30).
  Confirming the offset needs a debug build, which has not been done.
- Verification is single-box: one gfx1100 + gfx1030 machine, one kernel
  version.

The -EBUSY guard itself dates to commit a46a2cd103a8 ("drm/amdgpu: Add
GPUVM memory management functions for KFD") and is correct for the ioctl
path. The leak became reachable only once a caller started dropping the
last handle regardless of the return value.

Fixes: 52b29d73340d ("drm/amdkfd: Add per-process IDR for buffer handles")
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5672
Assisted-by: LLM checkpatch
Tested-by: Bocaj Gnuoy <bocajgnuoy@gmail.com>
Signed-off-by: Bocaj Gnuoy <bocajgnuoy@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 25 ++++++++++++++++---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  9 ++++---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c      | 21 ++++++++++++----
 4 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 1b7dc0d3963b..4ad843105443 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -326,7 +326,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 		uint64_t *offset, uint32_t flags, bool criu_resume);
 int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 		struct amdgpu_device *adev, struct kgd_mem *mem, void *drm_priv,
-		uint64_t *size);
+		uint64_t *size, bool force);
 int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(struct amdgpu_device *adev,
 					  struct kgd_mem *mem, void *drm_priv);
 int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 1e71829e0fc6..dc1fa664feca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1901,7 +1901,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 
 int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 		struct amdgpu_device *adev, struct kgd_mem *mem, void *drm_priv,
-		uint64_t *size)
+		uint64_t *size, bool force)
 {
 	struct amdkfd_process_info *process_info = mem->process_info;
 	unsigned long bo_size = mem->bo->tbo.base.size;
@@ -1922,9 +1922,26 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 	 */
 
 	if (mapped_to_gpu_memory > 0) {
-		pr_debug("BO VA 0x%llx size 0x%lx is still mapped.\n",
-				mem->va, bo_size);
-		return -EBUSY;
+		/*
+		 * Refusing to free a mapped BO is only meaningful while the
+		 * process can still unmap it. On process teardown (@force)
+		 * there is no such chance: the caller drops the last handle
+		 * to @mem regardless, so bailing out here leaks the BO onto
+		 * process_info->kfd_bo_list / userptr_inval_list. Those lists
+		 * are then destroyed non-empty in
+		 * amdgpu_amdkfd_gpuvm_destroy_cb(), leaving a BO in TTM's
+		 * eviction LRU whose bo_vas point into the freed amdgpu_vm.
+		 * The next client to trigger eviction deadlocks in
+		 * amdgpu_vm_bo_move(). Tear the mappings down instead - the
+		 * VM is going away right after us anyway.
+		 */
+		if (!force) {
+			pr_debug("BO VA 0x%llx size 0x%lx is still mapped.\n",
+				 mem->va, bo_size);
+			return -EBUSY;
+		}
+		pr_warn("Force-freeing BO VA 0x%llx size 0x%lx still mapped %u time(s)\n",
+			mem->va, bo_size, mapped_to_gpu_memory);
 	}
 
 	/* At this point the BO is guaranteed to be freed, so unpin the
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 6fd18488d5cf..0121c6dc77b3 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1219,7 +1219,7 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
 
 err_free:
 	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->adev, (struct kgd_mem *)mem,
-					       pdd->drm_priv, NULL);
+					       pdd->drm_priv, NULL, false);
 err_unlock:
 err_pdd:
 err_large_bar:
@@ -1262,7 +1262,8 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
 	}
 
 	ret = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev,
-				(struct kgd_mem *)mem, pdd->drm_priv, &size);
+				(struct kgd_mem *)mem, pdd->drm_priv, &size,
+				false);
 
 	/* If freeing the buffer failed, leave the handle in place for
 	 * clean-up during process tear-down.
@@ -1618,7 +1619,7 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
 
 err_free:
 	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, (struct kgd_mem *)mem,
-					       pdd->drm_priv, NULL);
+					       pdd->drm_priv, NULL, false);
 err_unlock:
 	mutex_unlock(&p->mutex);
 	return r;
@@ -2483,7 +2484,7 @@ static int criu_restore_memory_of_gpu(struct kfd_process_device *pdd,
 	if (idr_handle < 0) {
 		pr_err("Could not allocate idr\n");
 		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, *kgd_mem, pdd->drm_priv,
-						       NULL);
+						       NULL, false);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 0a7c1900da95..6d5126aa6fe7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -743,7 +743,7 @@ static void kfd_process_free_gpuvm(struct kgd_mem *mem,
 
 	amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(dev->adev, mem, pdd->drm_priv);
 	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->adev, mem, pdd->drm_priv,
-					       NULL);
+					       NULL, true);
 }
 
 static void kfd_process_free_gpuvm_map(struct kgd_mem *mem,
@@ -759,7 +759,7 @@ static void kfd_process_free_gpuvm_map(struct kgd_mem *mem,
 
 	amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(dev->adev, mem, pdd->drm_priv);
 	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->adev, mem, pdd->drm_priv,
-					       NULL);
+					       NULL, true);
 }
 
 /* kfd_process_alloc_gpuvm - Allocate GPU VM for the KFD process
@@ -814,7 +814,7 @@ static int kfd_process_alloc_gpuvm(struct kfd_process_device *pdd,
 
 err_map_mem:
 	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->adev, *mem, pdd->drm_priv,
-					       NULL);
+					       NULL, false);
 err_alloc_mem:
 	*mem = NULL;
 	*kptr = NULL;
@@ -1119,18 +1119,29 @@ static void kfd_process_device_free_bos(struct kfd_process_device *pdd)
 	 * local memory object
 	 */
 	idr_for_each_entry(&pdd->alloc_idr, mem, id) {
+		int r;
 
 		for (i = 0; i < p->n_pdds; i++) {
 			struct kfd_process_device *peer_pdd = p->pdds[i];
 
 			if (!peer_pdd->drm_priv)
 				continue;
+			/*
+			 * This can fail under memory pressure: unmapping has
+			 * to validate the page-table BOs first. Ignore it and
+			 * force the free below - the handle is dropped either
+			 * way, so a failed free would leak the BO onto the
+			 * process_info lists and poison the eviction LRU.
+			 */
 			amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
 				peer_pdd->dev->adev, mem, peer_pdd->drm_priv);
 		}
 
-		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, mem,
-						       pdd->drm_priv, NULL);
+		r = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, mem,
+							   pdd->drm_priv, NULL,
+							   true);
+		if (r)
+			pr_err("Failed to free BO on process teardown: %d\n", r);
 		kfd_process_device_remove_obj_handle(pdd, id);
 	}
 }
-- 
2.55.0

[PATCH v2] drm/amdkfd: don't leak BOs when process teardown can't unmap them

[Bocaj Gnuoy]

kfd_process_device_free_bos() discards the return value of both
amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu() and
amdgpu_amdkfd_gpuvm_free_memory_of_gpu(), then drops the idr handle
unconditionally.  That is fine as long as the free succeeds, but the unmap
has to validate the page-table BOs first (vm_validate_pt_pd_bos()), and
that allocates.  When the process is dying *because* memory is exhausted -
e.g. GTT pinned up to amdgpu.gttsize by a compute client that then aborted
on ENOMEM - the unmap fails with "failed to validate PT BOs",
mem->mapped_to_gpu_memory stays non-zero, and
amdgpu_amdkfd_gpuvm_free_memory_of_gpu() bails out with -EBUSY before it
removes the BO from process_info->kfd_bo_list.  The handle is dropped
anyway, so nothing will ever free that kgd_mem again.

The consequences are worse than a leak.  drm_release() ->
amdgpu_vm_fini() -> amdgpu_amdkfd_gpuvm_destroy_cb() then trips

  WARN_ON(!list_empty(&process_info->kfd_bo_list));
  WARN_ON(!list_empty(&process_info->userptr_inval_list));

and frees process_info regardless, while the leaked BO stays in TTM's
eviction LRU with bo_vas pointing into the just-freed amdgpu_vm.  A
subsequent amdgpu_gem_create_ioctl() that forces TTM eviction can then walk
into it and spin in amdgpu_vm_bo_move() -> _raw_spin_lock(), which produces
repeating rcu_preempt self-detected stalls, blocks reclaim-related work
(__lru_add_drain_all()) and eventually makes the machine unusable - within
about ten minutes here.  A recoverable ENOMEM becomes an unrecoverable
hang.

Note the free path itself does not allocate - it reserves the BO, detaches
the attachments and drops the references - so it can complete even when the
unmap could not.  Give amdgpu_amdkfd_gpuvm_free_memory_of_gpu() a @force
flag and set it on the teardown callers, which have no later chance to try
again.  The -EBUSY guard is kept for the ioctl paths, where userspace can
still unmap and retry, and where kfd_ioctl_free_memory_of_gpu() already
leaves the handle in place on failure.

Reported on: RX 6900 XT (gfx1030), amdgpu 3.64.0, kernel 7.1.6/7.1.8,
booted with amdgpu.gttsize=4096.  The failing allocation was on the Navi 21
at 0000:06:00.0, which enumerates as ROCm0; a gfx1100 is also present in
the same box but was not the device that ran out.

Tooling disclosure, per Documentation/process/generated-content.rst: this
bug was diagnosed and this patch written with the assistance of Claude Code
(claude-opus-5) in a single interactive session. The input was a kernel
trace captured on the reporting host - two WARNs out of
amdgpu_amdkfd_gpuvm_destroy_cb() during an aborting client's teardown,
followed by an unrecoverable deadlock in the TTM eviction path in the next
process to allocate - together with the request to find the cause and, if
possible, fix it. The assistant read the KFD BO lifecycle in the v7.1.8 and
v7.2 sources, identified the -EBUSY early return as the point at which the
BO is stranded on the process_info lists, and wrote both the diff and this
changelog. The diff was then rebased onto amd-staging-drm-next and the
call-site audit redone against that tree: one prototype, one definition and
eight call sites, all converted. The rebase caught a real miss - the tree
has gained kfd_process_free_gpuvm_map() alongside kfd_process_free_gpuvm(),
and the original hunk's trailing context matched the wrong one of the two.
Both are teardown-only (kfd_process_destroy_pdds() and the create_process()
error unwind), so both take force=true. checkpatch.pl was the only
additional analysis tool used.

Tested on a RX 6900 XT + RX 7900 XTX box, kernel 7.2.0-1-cachyos, booted
with amdgpu.gttsize=4096 so the GTT wall is reachable. A HIP-linked client
(llama.cpp with both the HIP and Vulkan backends present) was made to pin
host memory up to the 4 GiB cap and die on the resulting ENOMEM. Same
kernel version, same cap, same workload, same binary either side; the
patch is the only variable:

                              unpatched   patched
  peak GTT                      4.00 GiB   4.00 GiB
  "failed to validate PT BOs"         34         36
  exit                           139/SEGV   139/SEGV
  teardown WARNs                       2          0
  "Force-freeing BO VA"                0          9

Both sides died the same way, on SIGSEGV, so the difference is in teardown
and not in how the client failed.  The force-free count of 9 matched the 9
KFD allocations an earlier kprobe run measured for this client, consistent
with all of those allocations being reclaimed.

End to end: on the unpatched kernel, relaunching the same workload after
that abort is what hangs the machine - the new client's
amdgpu_gem_create_ioctl walks the eviction LRU into the orphaned object.
With the patch, a second run under heavier pressure (1904 "failed to
validate PT BOs", 35 force-freed BOs) again left zero WARNs, and the
relaunch then loaded in 12 seconds and served normally instead of
deadlocking. GTT returned to its 0.05 GiB idle baseline afterwards, so the
memory is genuinely reclaimed rather than merely not deadlocking. Across
four runs the client died variously on SIGABRT and SIGSEGV; the leak
tracked memory exhaustion at teardown, never the signal.

Note the WARNs that fire vary between runs: the originally reported crash
tripped :1608 and :1610 (kfd_bo_list, userptr_inval_list), while this
reproduction tripped :1608 and :1609 (kfd_bo_list, userptr_valid_list).
Both are the same skipped list_del_init(&mem->validate_list) - that node
serves whichever list the BO currently sits on, so the leak is
list-agnostic. The client also died on SIGSEGV rather than the abort seen
originally; the leak depends on memory being exhausted at teardown, not on
how the process died, which is consistent with SIGKILL alone never
reproducing it.

Limitations, per Documentation/process/coding-assistants.rst step 8:
- The deadlock itself was reproduced on the unpatched kernel, but its stack
  trace was not captured: the jammed workqueue takes journald down with it,
  so nothing reaches disk. Capturing it needs netconsole or a serial
  console. The teardown WARNs, which are the direct measure of the leak,
  are captured in full.
- The claim that the deadlocked spinlock lives inside the leaked object
  still rests on address arithmetic (lock at the WARNed structure + 0x30).
  Confirming the offset needs a debug build, which has not been done.
- Verification is single-box: one gfx1100 + gfx1030 machine, one kernel
  version.

The -EBUSY guard itself dates to commit a46a2cd103a8 ("drm/amdgpu: Add
GPUVM memory management functions for KFD") and is correct for the ioctl
path. The leak became reachable only once a caller started dropping the
last handle regardless of the return value.

Fixes: 52b29d73340d ("drm/amdkfd: Add per-process IDR for buffer handles")
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5672
Assisted-by: Claude:claude-opus-5 checkpatch
Signed-off-by: Bocaj Gnuoy <bocajgnuoy@gmail.com>
---

v2:
 - fix Assisted-by tag format per Documentation/process/coding-assistants.rst
   (v1 used a malformed 'LLM checkpatch' form)
 - drop self-applied Tested-by; the testing is described in the changelog
 - reword the comment on the forced path: what happens is attachment/VA
   teardown via kfd_mem_detach(), not the page-table unmap that failed
 - no functional change
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 27 ++++++++++++++++---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  9 ++++---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c      | 21 +++++++++++----
 4 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 1b7dc0d3963b..4ad843105443 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -326,7 +326,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 		uint64_t *offset, uint32_t flags, bool criu_resume);
 int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 		struct amdgpu_device *adev, struct kgd_mem *mem, void *drm_priv,
-		uint64_t *size);
+		uint64_t *size, bool force);
 int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(struct amdgpu_device *adev,
 					  struct kgd_mem *mem, void *drm_priv);
 int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 1e71829e0fc6..aee2edbf1ec8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1901,7 +1901,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 
 int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 		struct amdgpu_device *adev, struct kgd_mem *mem, void *drm_priv,
-		uint64_t *size)
+		uint64_t *size, bool force)
 {
 	struct amdkfd_process_info *process_info = mem->process_info;
 	unsigned long bo_size = mem->bo->tbo.base.size;
@@ -1922,9 +1922,28 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 	 */
 
 	if (mapped_to_gpu_memory > 0) {
-		pr_debug("BO VA 0x%llx size 0x%lx is still mapped.\n",
-				mem->va, bo_size);
-		return -EBUSY;
+		/*
+		 * Refusing to free a mapped BO is only meaningful while the
+		 * process can still unmap it. On process teardown (@force)
+		 * there is no such chance: the caller drops the last handle
+		 * to @mem regardless, so bailing out here leaks the BO onto
+		 * process_info->kfd_bo_list / userptr_inval_list. Those lists
+		 * are then destroyed non-empty in
+		 * amdgpu_amdkfd_gpuvm_destroy_cb(), leaving a BO in TTM's
+		 * eviction LRU whose bo_vas point into the freed amdgpu_vm.
+		 * A subsequent client that triggers eviction can then spin in
+		 * amdgpu_vm_bo_move(). Drop the attachments instead: the
+		 * kfd_mem_detach() calls below release the bo_va references
+		 * without needing the page-table update that just failed, and
+		 * the VM is torn down right after us anyway.
+		 */
+		if (!force) {
+			pr_debug("BO VA 0x%llx size 0x%lx is still mapped.\n",
+				 mem->va, bo_size);
+			return -EBUSY;
+		}
+		pr_warn("Force-freeing BO VA 0x%llx size 0x%lx still mapped %u time(s)\n",
+			mem->va, bo_size, mapped_to_gpu_memory);
 	}
 
 	/* At this point the BO is guaranteed to be freed, so unpin the
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 6fd18488d5cf..0121c6dc77b3 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1219,7 +1219,7 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
 
 err_free:
 	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->adev, (struct kgd_mem *)mem,
-					       pdd->drm_priv, NULL);
+					       pdd->drm_priv, NULL, false);
 err_unlock:
 err_pdd:
 err_large_bar:
@@ -1262,7 +1262,8 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
 	}
 
 	ret = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev,
-				(struct kgd_mem *)mem, pdd->drm_priv, &size);
+				(struct kgd_mem *)mem, pdd->drm_priv, &size,
+				false);
 
 	/* If freeing the buffer failed, leave the handle in place for
 	 * clean-up during process tear-down.
@@ -1618,7 +1619,7 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
 
 err_free:
 	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, (struct kgd_mem *)mem,
-					       pdd->drm_priv, NULL);
+					       pdd->drm_priv, NULL, false);
 err_unlock:
 	mutex_unlock(&p->mutex);
 	return r;
@@ -2483,7 +2484,7 @@ static int criu_restore_memory_of_gpu(struct kfd_process_device *pdd,
 	if (idr_handle < 0) {
 		pr_err("Could not allocate idr\n");
 		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, *kgd_mem, pdd->drm_priv,
-						       NULL);
+						       NULL, false);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 0a7c1900da95..6d5126aa6fe7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -743,7 +743,7 @@ static void kfd_process_free_gpuvm(struct kgd_mem *mem,
 
 	amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(dev->adev, mem, pdd->drm_priv);
 	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->adev, mem, pdd->drm_priv,
-					       NULL);
+					       NULL, true);
 }
 
 static void kfd_process_free_gpuvm_map(struct kgd_mem *mem,
@@ -759,7 +759,7 @@ static void kfd_process_free_gpuvm_map(struct kgd_mem *mem,
 
 	amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(dev->adev, mem, pdd->drm_priv);
 	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->adev, mem, pdd->drm_priv,
-					       NULL);
+					       NULL, true);
 }
 
 /* kfd_process_alloc_gpuvm - Allocate GPU VM for the KFD process
@@ -814,7 +814,7 @@ static int kfd_process_alloc_gpuvm(struct kfd_process_device *pdd,
 
 err_map_mem:
 	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->adev, *mem, pdd->drm_priv,
-					       NULL);
+					       NULL, false);
 err_alloc_mem:
 	*mem = NULL;
 	*kptr = NULL;
@@ -1119,18 +1119,29 @@ static void kfd_process_device_free_bos(struct kfd_process_device *pdd)
 	 * local memory object
 	 */
 	idr_for_each_entry(&pdd->alloc_idr, mem, id) {
+		int r;
 
 		for (i = 0; i < p->n_pdds; i++) {
 			struct kfd_process_device *peer_pdd = p->pdds[i];
 
 			if (!peer_pdd->drm_priv)
 				continue;
+			/*
+			 * This can fail under memory pressure: unmapping has
+			 * to validate the page-table BOs first. Ignore it and
+			 * force the free below - the handle is dropped either
+			 * way, so a failed free would leak the BO onto the
+			 * process_info lists and poison the eviction LRU.
+			 */
 			amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
 				peer_pdd->dev->adev, mem, peer_pdd->drm_priv);
 		}
 
-		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, mem,
-						       pdd->drm_priv, NULL);
+		r = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, mem,
+							   pdd->drm_priv, NULL,
+							   true);
+		if (r)
+			pr_err("Failed to free BO on process teardown: %d\n", r);
 		kfd_process_device_remove_obj_handle(pdd, id);
 	}
 }
-- 
2.55.0

Re: [PATCH v2] drm/amdkfd: don't leak BOs when process teardown can't unmap them

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Internal BO allocations (like IB and CWSR) are leaked during error unwinding because `kfd_process_alloc_gpuvm` uses `force = false` and permanently loses the reference.
- [High] The force-free path can still fail under memory pressure due to `GFP_KERNEL` allocations, silently bypassing teardown WARNs and causing the same Use-After-Free deadlock.
- [High] `amdgpu_amdkfd_gpuvm_free_memory_of_gpu` aborts early and leaks BOs if a signal is pending during error unwinding.
--

--- Patch [1]: [PATCH v2] drm/amdkfd: don't leak BOs when process teardown can't unmap them ---
commit d455d7ae97544007b84e3463aa9bc5371a4a8935
Author: Bocaj Gnuoy <bocajgnuoy@gmail.com>

drm/amdkfd: don't leak BOs when process teardown can't unmap them

This commit fixes a memory leak and subsequent TTM eviction deadlock that
occurs when BO unmapping fails during process teardown due to memory
exhaustion. It adds a force flag to amdgpu_amdkfd_gpuvm_free_memory_of_gpu()
to bypass the mapping check and detach attachments instead of aborting.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5672

> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 0a7c1900da959..6d5126aa6fe74 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c

[ ... ]

> @@ -814,7 +814,7 @@ static int kfd_process_alloc_gpuvm(struct kfd_process_device *pdd,
>  
>  err_map_mem:
>  	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->adev, *mem, pdd->drm_priv,
> -					       NULL);
> +					       NULL, false);

[Severity: High]
This isn't a bug introduced by this patch, but does this error unwinding path
leak internal BO allocations?

If amdgpu_amdkfd_gpuvm_free_memory_of_gpu() fails here due to the memory being
exhausted during unmapping (similar to the user-space BOs), the force = false
flag means it will safely return -EBUSY but abort the cleanup.

Since the pointers are then unconditionally cleared:

err_alloc_mem:
	*mem = NULL;
	*kptr = NULL;

Does the reference to the BO get permanently lost, causing the exact same
use-after-free sequence this patch is addressing?

>  err_alloc_mem:
>  	*mem = NULL;
>  	*kptr = NULL;

[ ... ]

> @@ -1119,18 +1119,29 @@ static void kfd_process_device_free_bos(struct kfd_process_device *pdd)

[ ... ]

>  		}
>  
> -		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, mem,
> -						       pdd->drm_priv, NULL);
> +		r = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->adev, mem,
> +							   pdd->drm_priv, NULL,
> +							   true);
> +		if (r)
> +			pr_err("Failed to free BO on process teardown: %d\n", r);

[Severity: High]
This is a pre-existing issue, but does this still leak BOs and cause a TTM
eviction deadlock if amdgpu_amdkfd_gpuvm_free_memory_of_gpu() fails for other
reasons?

Looking at amdgpu_amdkfd_gpuvm_free_memory_of_gpu(), it can still fail during
reservation:

	ret = reserve_bo_and_cond_vms(mem, NULL, BO_VM_ALL, &ctx);
	if (unlikely(ret))
		return ret;

This reservation can fail under memory exhaustion if the GFP_KERNEL allocations
in drm_exec_init() fail. Since the caller drops the IDR handle regardless of
the error, will the dangling bo_va pointers still poison the eviction LRU?

[Severity: High]
This is also a pre-existing issue, but what happens if a signal is pending
during process teardown?

The same reservation path in reserve_bo_and_cond_vms() uses an interruptible
wait:

	drm_exec_init(&ctx->exec, DRM_EXEC_INTERRUPTIBLE_WAIT |
		      DRM_EXEC_IGNORE_DUPLICATES, 0);

If a signal (e.g. SIGKILL) is pending, this fails with -ERESTARTSYS.

Does this cause amdgpu_amdkfd_gpuvm_free_memory_of_gpu() to skip the remaining
cleanup and leak the attachments, leading to the same use-after-free sequence
when the handle is dropped?

>  		kfd_process_device_remove_obj_handle(pdd, id);
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822144402.929677-1-bocajgnuoy@gmail.com?part=1

Re: [PATCH v2] drm/amdkfd: don't leak BOs when process teardown can't unmap them

[Bocaj Gnuoy]

Correcting a factual error in my own changelog before anyone spends
review time on it.

The changelog states:

  "Note the free path itself does not allocate - it reserves the BO,
   detaches the attachments and drops the references - so it can
   complete even when the unmap could not."

The first clause is wrong. Immediately after the -EBUSY bypass,
amdgpu_amdkfd_gpuvm_free_memory_of_gpu() calls:

	ret = reserve_bo_and_cond_vms(mem, NULL, BO_VM_ALL, &ctx);
	if (unlikely(ret))
		return ret;

reserve_bo_and_cond_vms() uses drm_exec_prepare_obj(), and drm_exec
allocates its object array with kvmalloc_array()/kvrealloc(GFP_KERNEL)
and returns -ENOMEM on failure; drm_exec_prepare_obj() also calls
dma_resv_reserve_fences(). So the free path can allocate, and can fail
with -ENOMEM.

Bounded consequence
-------------------

The forced free can therefore still fail, after which
kfd_process_device_free_bos() drops the idr handle regardless - the
same ownership violation the patch addresses, one level deeper.

This is not detectable through the teardown WARNs. The BO is removed
from process_info's lists before the reservation is attempted:

	/* Make sure restore workers don't access the BO any more */
	mutex_lock(&process_info->lock);
	if (!list_empty(&mem->validate_list))
		list_del_init(&mem->validate_list);
	mutex_unlock(&process_info->lock);

	ret = reserve_bo_and_cond_vms(mem, NULL, BO_VM_ALL, &ctx);
	if (unlikely(ret))
		return ret;

That ordering is deliberate, so reordering it is not a fix. The
consequence is that if the reservation fails, the attachments are
never detached and the free does not complete, but the lists that
amdgpu_amdkfd_gpuvm_destroy_cb() checks are already empty. The stale
lifetime condition implicated in the deadlock can therefore survive
without the WARNs firing. I have not driven that particular residual
failure through the eviction path and observed the deadlock, so I am
describing a reachable state, not a reproduced one.

Scope of the change
-------------------

The underlying violation is not introduced by this patch. Upstream
today can already reach it:

  mapped_to_gpu_memory == 0
    -> delist
    -> reservation fails
    -> free returns error
    -> teardown discards the handle

This patch additionally permits:

  mapped_to_gpu_memory > 0 after a failed unmap
    -> bypass -EBUSY
    -> delist
    -> reservation fails
    -> free returns error
    -> teardown discards the handle

So it widens the set of states that can reach the violation rather
than creating it. That is not offered as a justification, only as
scope.

What the testing does and does not show
---------------------------------------

The pr_err() this patch adds to kfd_process_device_free_bos() is the
only instrumentation in the patch that directly observes a
post-delisting free failure. It did not fire in any run, including
one with 1904 "failed to validate PT BOs" and 35 forced frees.

That is consistent with the drm_exec allocations being far smaller
than the page-table validation that failed, and therefore satisfiable
under the same pressure. It is a probability argument, not an
invariant.

The accurate evidentiary statement is: the fix reliably completed in
the tested pressure regime, but the implementation does not provide an
invariant guaranteeing teardown completion under arbitrary allocation
failure.

What this does and does not change
----------------------------------

The reproduced failure is unaffected: the unmap fails in
vm_validate_pt_pd_bos(), mapped_to_gpu_memory stays non-zero, the free
returns -EBUSY before list_del_init(&mem->validate_list), and the BO
is stranded. This patch removes that, and in every observed forced
teardown the cleanup completed and the machine-killing relaunch
stopped happening.

What the error invalidates is the completeness claim, not the safety
argument for forcing. Bypassing -EBUSY on irreversible teardown and
detaching the bo_vas when the reservation succeeds is still supported
by the evidence. What I can no longer claim is that the cleanup is
guaranteed to succeed merely because the failing page-table operation
was skipped.

v3 will correct the changelog and state this as an explicit
limitation. If the residual allocation failure should be repaired
rather than documented, there appear to be several possible
directions with different locking and lifetime implications - a
retry, a reservation path that does not allocate, or deferring
ownership so a later pass can free the BO, among others. I would
rather have maintainer guidance on which is wanted than guess at a
lifetime-ownership decision.

Thanks,
Bocaj

Re: [PATCH] drm/amdkfd: don't leak BOs when process teardown can't unmap them

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch misses applying force=true to the error unwind path of kfd_process_alloc_gpuvm(), which can permanently leak internal BOs on unmap failure.
- [High] Interruptible lock waiting in reserve_bo_and_cond_vms causes early return on pending signals, bypassing the forced free and silently leaking the BO.
--

--- Patch [1]: [PATCH] drm/amdkfd: don't leak BOs when process teardown can't unmap them ---
commit 01e590b61624d149d5da9ac22540c56bed4f9a55
Author: Bocaj Gnuoy <bocajgnuoy@gmail.com>
Subject: drm/amdkfd: don't leak BOs when process teardown can't unmap them

This commit addresses a memory leak and subsequent TTM eviction deadlock
that occurs when a process runs out of memory during teardown. It adds a
force flag to amdgpu_amdkfd_gpuvm_free_memory_of_gpu() to ensure BOs are
freed even if unmapping fails. The early -EBUSY return is retained for ioctl
paths where userspace can retry.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5672

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 1e71829e0fc6b..dc1fa664fecac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> [ ... ]
> @@ -1922,9 +1922,26 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>  	 */
>  
>  	if (mapped_to_gpu_memory > 0) {
> -		pr_debug("BO VA 0x%llx size 0x%lx is still mapped.\n",
> -				mem->va, bo_size);
> -		return -EBUSY;
> +		/*
> +		 * Refusing to free a mapped BO is only meaningful while the
> +		 * process can still unmap it. On process teardown (@force)
> +		 * there is no such chance: the caller drops the last handle
> +		 * to @mem regardless, so bailing out here leaks the BO onto
> +		 * process_info->kfd_bo_list / userptr_inval_list. Those lists
> +		 * are then destroyed non-empty in
> +		 * amdgpu_amdkfd_gpuvm_destroy_cb(), leaving a BO in TTM's
> +		 * eviction LRU whose bo_vas point into the freed amdgpu_vm.
> +		 * The next client to trigger eviction deadlocks in
> +		 * amdgpu_vm_bo_move(). Tear the mappings down instead - the
> +		 * VM is going away right after us anyway.
> +		 */
> +		if (!force) {
> +			pr_debug("BO VA 0x%llx size 0x%lx is still mapped.\n",
> +				 mem->va, bo_size);
> +			return -EBUSY;
> +		}
> +		pr_warn("Force-freeing BO VA 0x%llx size 0x%lx still mapped %u time(s)\n",
> +			mem->va, bo_size, mapped_to_gpu_memory);
>  	}
>  
>  	/* At this point the BO is guaranteed to be freed, so unpin the

[Severity: High]
This is a pre-existing issue, but can amdgpu_amdkfd_gpuvm_free_memory_of_gpu()
still silently leak the BO if a signal interrupts the lock wait?

Later in this function, the BO is removed from the tracking list before an
interruptible lock wait is performed:

amdgpu_amdkfd_gpuvm_free_memory_of_gpu()
    mutex_lock(&process_info->lock);
    if (!list_empty(&mem->validate_list))
        list_del_init(&mem->validate_list);
    mutex_unlock(&process_info->lock);

    ret = reserve_bo_and_cond_vms(mem, NULL, BO_VM_ALL, &ctx);
    if (unlikely(ret))
        return ret;

If reserve_bo_and_cond_vms() returns early (e.g., -ERESTARTSYS) due to a
pending signal, the function bails out. Because the BO was already removed
from validate_list, it is left orphaned.

Additionally, looking at the related caller kfd_process_free_gpuvm_map(),
the map pointer is cleared before the free is attempted:

kfd_process_free_gpuvm_map()
    if (map && !iosys_map_is_null(map)) {
        amdgpu_amdkfd_gpuvm_unmap_bo_from_kernel(mem);
        iosys_map_clear(map);
    }

If the free is aborted by a signal, subsequent teardown passes will skip
this map and ignore this BO. Does this sequence also create a risk of a
silent memory leak and TTM eviction deadlock when userspace triggers an
error and receives a concurrent signal?

> [ ... ]
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 0a7c1900da959..6d5126aa6fe74 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> [ ... ]
> @@ -814,7 +814,7 @@ static int kfd_process_alloc_gpuvm(struct kfd_process_device *pdd,
>  
>  err_map_mem:
>  	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->adev, *mem, pdd->drm_priv,
> -					       NULL);
> +					       NULL, false);
>  err_alloc_mem:
>  	*mem = NULL;
>  	*kptr = NULL;

[Severity: High]
This is a pre-existing issue, but should the error unwind path in
kfd_process_alloc_gpuvm() use force = true?

When kfd_process_alloc_gpuvm() hits an error and jumps to err_map_mem, it
calls amdgpu_amdkfd_gpuvm_free_memory_of_gpu() with force = false. It then
immediately zeroes out the *mem pointer.

If the free fails (for instance, an unmap failure under memory exhaustion
where force = false causes an -EBUSY return), this BO appears to be
permanently leaked since *mem is cleared and no later retry is possible.

Could this lead to the same unrecoverable TTM eviction deadlock that this
patch is fixing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822142521.899098-1-bocajgnuoy@gmail.com?part=1