Re: [PATCH] drm/amdkfd: fix use-after-free in create_process() error path
From: Kuehling, Felix
Date: Fri Nov 13 2026 - 18:22:24 EST
On 2026-11-08 05:56, Hongyan Xu wrote:
create_process() publishes the process in kfd_processes_table before its
last failing call (alloc_notifier() must find it there), and its error path
frees it with a bare kfree(), ignoring the refcount. find_process() takes
its reference inside the sRCU critical section but callers use the process
after leaving it, so synchronize_srcu() misses them, and the error path
cannot unref once either: it holds two (kref_init() and the one keeping
free_notifier() from starting kfd_process_wq_release()).
Release the process as soon as it is visible: take the lead thread
reference right after lead_thread is assigned, run ida_init() before
mmu_notifier_get(), so the release work is safe on a half-built object.
The published error paths unlink with hash_del_rcu(), synchronize_srcu()
and drop both references, so the last one runs kfd_process_wq_release().
Both labels hold exactly two references, so nothing is freed early and no
path falls into the unwind the unpublished labels keep, now with
put_task_struct()). Restore pqm_uninit() (kfd_process_queue_manager.c):
pqm_init()'s 128 byte queue slot bitmap leaked per failure. Match
kfd_debugfs_remove_process() by process, not pid.
Fixes: 3248b6d3cbcd ("drm/amdkfd: Use mmu_notifier_get")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Hongyan Xu <getshell@xxxxxxxxxx>
---
drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c | 12 ++++++-
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 46 ++++++++++++++++++++++--
2 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c b/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
index 7d4e07452cdb..a17a3a307266 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
@@ -33,6 +33,7 @@ static struct list_head procs;
struct debugfs_proc_entry {
struct list_head list;
struct dentry *proc_dentry;
+ struct kfd_process *process;
Hi Hongyan,
Thank you for working on this. It looks like your patch is based on an outdated version of the branch. Current master branch already has a fix for this part of your patch:
commit 325de6bf3328efd5acf5072337fc196928753de8
Author: Zhu Lingshan <lingshan.zhu@xxxxxxx>
Date: Wed Jul 1 11:03:09 2026 +0800
drm/amdkfd: expose pasid of secondary contexts by debugfs
Current kfd debugfs interfaces only expose pasid
of the primary process, this commit exposes
pasid of secondary contexts by debugfs
Just like entries under sysfs,
the secondary contexts are named as
context_<id> under its primary kfd process.
The layout:
/sys/kernel/debug/kfd/proc# tree
.
└── 5802
├── context_0
│ ├── pasid_1025
│ └── pasid_63266
├── context_1
│ ├── pasid_1025
│ └── pasid_63266
├── pasid_1025
└── pasid_63266
Another fix is, kfd_debugfs_add_process may fail,
this commit change it to return a meaningful
value other than void
Signed-off-by: Zhu Lingshan <lingshan.zhu@xxxxxxx>
Reviewed-by: Felix Kuehling <felix.kuehling@xxxxxxx>
Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
pid_t pid;
};
@@ -151,6 +152,7 @@ void kfd_debugfs_add_process(struct kfd_process *p)
return;
list_add(&entry->list, &procs);
+ entry->process = p;
entry->pid = p->lead_thread->pid;
snprintf(name, MAX_DEBUGFS_FILENAME_LEN, "%d",
(int)entry->pid);
@@ -174,9 +176,17 @@ void kfd_debugfs_remove_process(struct kfd_process *p)
{
struct debugfs_proc_entry *entry, *next;
+ /*
+ * Match the entry that was added for this process and not one that
+ * merely has the same pid: kfd_process_wq_release() also runs for a
+ * process that failed in create_process() and never got an entry,
+ * and after an exec(3) the old and the new primary kfd_process of a
+ * process have the same lead thread while a file descriptor keeps
+ * the old one alive.
+ */
mutex_lock(&kfd_processes_mutex);
list_for_each_entry_safe(entry, next, &procs, list) {
- if (entry->pid != p->lead_thread->pid)
+ if (entry->process != p)
continue;
debugfs_remove_recursive(entry->proc_dentry);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 12e24fbf8c46..f118f7f03de7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1600,6 +1600,11 @@ struct kfd_process *create_process(const struct task_struct *thread, bool primar
mutex_init(&process->mutex);
process->mm = thread->mm;
process->lead_thread = thread->group_leader;
+ /*
+ * Take the reference on the lead thread before the process can be
+ * released through kfd_process_wq_release(), which drops it again.
+ */
+ get_task_struct(process->lead_thread);
process->n_pdds = 0;
process->queues_paused = false;
@@ -1648,13 +1653,17 @@ struct kfd_process *create_process(const struct task_struct *thread, bool primar
*/
if (primary) {
process->context_id = KFD_CONTEXT_ID_PRIMARY;
+ /*
+ * Must be initialized before the process can be given to
+ * kfd_process_wq_release(), which destroys it.
+ */
+ ida_init(&process->id_table);
mn = mmu_notifier_get(&kfd_process_mmu_notifier_ops, process->mm);
if (IS_ERR(mn)) {
err = PTR_ERR(mn);
goto err_register_notifier;
}
BUG_ON(mn != &process->mmu_notifier);
- ida_init(&process->id_table);
}
err = kfd_process_alloc_id(process);
@@ -1664,7 +1673,6 @@ struct kfd_process *create_process(const struct task_struct *thread, bool primar
}
kfd_unref_process(process);
- get_task_struct(process->lead_thread);
INIT_WORK(&process->debug_event_workarea, debug_event_write_work_handler);
@@ -1673,8 +1681,39 @@ struct kfd_process *create_process(const struct task_struct *thread, bool primar
err_alloc_id:
kfd_process_free_id(process);
err_register_notifier:
+ /*
+ * The process is already in kfd_processes_table, so find_process()
+ * may have handed it out with a reference taken on it. Unlink it
+ * and wait for the readers that are still walking the table before
+ * dropping the references held here: afterwards no new reference
+ * can be taken and the ones already taken keep the process alive.
+ * The memory is then freed by kfd_process_wq_release() when the
+ * last reference is dropped.
+ */
hash_del_rcu(&process->kfd_processes);
- svm_range_list_fini(process);
+ synchronize_srcu(&kfd_processes_srcu);
+
+ /*
+ * kfd_process_wq_release() frees the memory, but it does not
+ * destroy the queue manager: on the normal path that is done by
+ * kfd_process_notifier_release_internal(), which neither of these
+ * two error paths can reach, because no mmu notifier was
+ * registered for the process. The process is unlinked and no
+ * reader can reach the pqm any more, so destroy it here, before
+ * the references are dropped.
+ */
+ pqm_uninit(&process->pqm);
+
+ /*
+ * Drop the reference that keeps a failing free_notifier() from
+ * starting kfd_process_wq_release() and the one taken by
+ * kref_init().
+ */
+ kfd_unref_process(process);
+ kfd_unref_process(process);
+
+ return ERR_PTR(err);
+
err_init_svm_range_list:
kfd_process_free_outstanding_kfd_bos(process);
kfd_process_destroy_pdds(process);
@@ -1684,6 +1723,7 @@ struct kfd_process *create_process(const struct task_struct *thread, bool primar
kfd_event_free_process(process);
err_event_init:
mutex_destroy(&process->mutex);
+ put_task_struct(process->lead_thread);
The changes in kfd_process.c look reasonable. I'll try to apply them without the changes in kfd_debugfs.c.
Regards,
Felix
kfree(process);
err_alloc_process:
return ERR_PTR(err);