[PATCH] drm/amdkfd: fix use-after-free in create_process() error path

[PATCH] drm/amdkfd: fix use-after-free in create_process() error path

[Hongyan Xu]

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@vger.kernel.org
Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
---
 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