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: [email protected]
Signed-off-by: Hongyan Xu <[email protected]>
---
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;
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);
kfree(process);
err_alloc_process:
return ERR_PTR(err);
--
2.50.1.windows.1
