[PATCH v1] drm/amdkfd: Fix use-after-free in the process creation error path

[PATCH v1] drm/amdkfd: Fix use-after-free in the process creation error path

[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

Re: [PATCH v1] drm/amdkfd: Fix use-after-free in the process creation error path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] amdkfd: incomplete fix for UAF in create_process() error path
--

commit dcdae9cca3430fa247abfc570505faf612c7c75d
Author: Binbin Deng <18983559317@163.com>

drm/amdkfd: Fix use-after-free in the process creation error path

This commit attempts to fix a use-after-free race during process creation
by adding an SRCU grace period wait in the error path after removing the
process from the hash table.

> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1761,6 +1761,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);

[Severity: High]
Does this synchronize_srcu() guarantee no threads are accessing the object
when we eventually reach kfree(process) at the end of the error path?

If a concurrent reader finds the process before it is removed from the hash
table, it will increment the reference count and then immediately drop the
SRCU lock:

kfd_lookup_process_by_mm() {
        ...
        p = find_process_by_mm(mm);
        if (p)
                kref_get(&p->ref);

        srcu_read_unlock(&kfd_processes_srcu, idx);
        ...
}

Because the concurrent reader has already dropped the SRCU lock,
synchronize_srcu() will return immediately.

The create_process() error path then continues tearing down the state and
eventually reaches the err_event_init label:

create_process() {
        ...
err_event_init:
        mutex_destroy(&process->mutex);
        kfree(process);
        ...
}

This unconditionally frees the process memory, completely bypassing the
kref mechanism.

Could this cause a use-after-free if the concurrent reader is still actively
holding and using the pointer?

>  	svm_range_list_fini(process);
>  err_init_svm_range_list:
>  	kfd_process_free_outstanding_kfd_bos(process);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009132619.40151-1-18983559317@163.com?part=1

Re: [PATCH v1] drm/amdkfd: Fix use-after-free in the process creation error path

[Kuehling, Felix]

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