mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v1] drm/amdkfd: Fix use-after-free in the process creation error path
@ 2026-10-09 13:26 Binbin Deng
  2026-10-09 14:38 ` Kuehling, Felix
  0 siblings, 1 reply; 2+ messages in thread
From: Binbin Deng @ 2026-10-09 13:26 UTC (permalink / raw)
  To: Felix.Kuehling, alexander.deucher, christian.koenig, airlied, simona
  Cc: amd-gfx, dri-devel, linux-kernel, Binbin Deng

create_process() publishes the new process in the global hash table
before the steps that can still fail:

	/* alloc_notifier needs to find the process in the hash table */
	hash_add_rcu(kfd_processes_table, &process->kfd_processes,
			(uintptr_t)process->mm);

If a later step fails (mmu_notifier_get() with a pending signal, or
kfd_process_alloc_id()), the error path removes and frees it:

err_register_notifier:
	hash_del_rcu(&process->kfd_processes);
	svm_range_list_fini(process);
	...
	kfree(process);

hash_del_rcu() only unlinks the node; readers that entered an SRCU
read-side critical section before the deletion can still be walking
the hash bucket and using the node.  The unconditional kfree() that
follows is not covered by any SRCU grace period, so such a reader can
obtain and dereference freed memory.

The readers are the GPU interrupt and ioctl lookup helpers, which run
under srcu_read_lock(&kfd_processes_srcu) only:
kfd_lookup_process_by_pasid() iterates the whole table with
hash_for_each_rcu and dereferences p->pdds[] of every entry;
kfd_lookup_process_by_id() and kfd_lookup_process_by_mm() match and
return the process for further use.

The normal removal path (kfd_process_table_remove()) already executes
synchronize_srcu(&kfd_processes_srcu) after hash_del_rcu(); only the
error rollback path misses it.  The race is reachable by an unprivileged
GPU compute user: a thread whose process creation fails (e.g. ioctl
interrupted by SIGKILL at mmu_notifier_get) while another thread is
servicing a GPU interrupt that walks the process table.

Fix this by waiting for an SRCU grace period in the error path after
unlinking the process, before the object is freed.

Fixes: 0029cab3146a ("drm/amdkfd: fix a use after free race with mmu_notifer unregister")
Signed-off-by: Binbin Deng <18983559317@163.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 0a7c1900da95..8d0e70a256f7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1747,6 +1747,7 @@ struct kfd_process *create_process(const struct task_struct *thread, bool primar
 	kfd_process_free_id(process);
 err_register_notifier:
 	hash_del_rcu(&process->kfd_processes);
+	synchronize_srcu(&kfd_processes_srcu);
 	svm_range_list_fini(process);
 err_init_svm_range_list:
 	kfd_process_free_outstanding_kfd_bos(process);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v1] drm/amdkfd: Fix use-after-free in the process creation error path
  2026-10-09 13:26 [PATCH v1] drm/amdkfd: Fix use-after-free in the process creation error path Binbin Deng
@ 2026-10-09 14:38 ` Kuehling, Felix
  0 siblings, 0 replies; 2+ messages in thread
From: Kuehling, Felix @ 2026-10-09 14:38 UTC (permalink / raw)
  To: Binbin Deng, alexander.deucher, christian.koenig, airlied, simona
  Cc: amd-gfx, dri-devel, linux-kernel

On 2026-10-09 09:26, Binbin Deng wrote:
> [Some people who received this message don't often get email from 18983559317@163.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> create_process() publishes the new process in the global hash table
> before the steps that can still fail:
>
>          /* alloc_notifier needs to find the process in the hash table */
>          hash_add_rcu(kfd_processes_table, &process->kfd_processes,
>                          (uintptr_t)process->mm);
>
> If a later step fails (mmu_notifier_get() with a pending signal, or
> kfd_process_alloc_id()), the error path removes and frees it:
>
> err_register_notifier:
>          hash_del_rcu(&process->kfd_processes);
>          svm_range_list_fini(process);
>          ...
>          kfree(process);
>
> hash_del_rcu() only unlinks the node; readers that entered an SRCU
> read-side critical section before the deletion can still be walking
> the hash bucket and using the node.  The unconditional kfree() that
> follows is not covered by any SRCU grace period, so such a reader can
> obtain and dereference freed memory.
>
> The readers are the GPU interrupt and ioctl lookup helpers, which run
> under srcu_read_lock(&kfd_processes_srcu) only:
> kfd_lookup_process_by_pasid() iterates the whole table with
> hash_for_each_rcu and dereferences p->pdds[] of every entry;
> kfd_lookup_process_by_id() and kfd_lookup_process_by_mm() match and
> return the process for further use.
>
> The normal removal path (kfd_process_table_remove()) already executes
> synchronize_srcu(&kfd_processes_srcu) after hash_del_rcu(); only the
> error rollback path misses it.  The race is reachable by an unprivileged
> GPU compute user: a thread whose process creation fails (e.g. ioctl
> interrupted by SIGKILL at mmu_notifier_get) while another thread is
> servicing a GPU interrupt that walks the process table.
>
> Fix this by waiting for an SRCU grace period in the error path after
> unlinking the process, before the object is freed.

I don't think that's sufficient. Typically the lookup functions get a 
refcount of the kfd_process under the srcu read lock. Even after 
srcu_synchronize returns, other threads can be holding a reference. I 
think we need to use kfd_unref_process to free the process after it has 
been added to the hash table, instead of doing the cleanup manually.

I see another problem: kfd_process_alloc_id can fail after registering 
the MMU notifier. That should be moved before the registration.

Regards,
   Felix


>
> Fixes: 0029cab3146a ("drm/amdkfd: fix a use after free race with mmu_notifer unregister")
> Signed-off-by: Binbin Deng <18983559317@163.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 0a7c1900da95..8d0e70a256f7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1747,6 +1747,7 @@ struct kfd_process *create_process(const struct task_struct *thread, bool primar
>          kfd_process_free_id(process);
>   err_register_notifier:
>          hash_del_rcu(&process->kfd_processes);
> +       synchronize_srcu(&kfd_processes_srcu);
>          svm_range_list_fini(process);
>   err_init_svm_range_list:
>          kfd_process_free_outstanding_kfd_bos(process);
> --
> 2.43.0

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-09 14:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-09 13:26 [PATCH v1] drm/amdkfd: Fix use-after-free in the process creation error path Binbin Deng
2026-10-09 14:38 ` Kuehling, Felix

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®