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

[sashiko-bot@kernel.org]

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