[PATCH] amdkfd: fix doorbell allocation race creating duplicate allocations

[PATCH] amdkfd: fix doorbell allocation race creating duplicate allocations

[Henry Martin]

The check-and-allocate sequence on pdd->qpd.proc_doorbells is
lockless at every call site of kfd_alloc_process_doorbells():
kfd_get_process_doorbells() (mmap of /dev/kfd), kfd_ioctl_create_queue()
and the CRIU restore path each test the pointer under different
locking (or none), so two racing threads can both observe NULL and
each allocate a doorbell BO and bitmap, orphaning one allocation and
handing the two mappings different backing pages.

Move the serialization into kfd_alloc_process_doorbells() itself:
take kfd->doorbell_mutex and re-check under it, so all three call
sites are covered regardless of their own locking.  The existing
doorbell_mutex users (kfd_get_kernel_doorbell() /
kfd_release_kernel_doorbell()) only guard the device-global
kfd->doorbell_bitmap in short non-sleeping sections and never call
into this path, so there is no nesting.

This vulnerability was discovered by Tencent CodeBuddy Security.

Fixes: 428542d91772 ("drm/amd/display: Setup for mmhubbub3_warmup_mcif with big buffer")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
index fdcf7f2d1b5b4..07f881573837e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
@@ -257,12 +257,21 @@ int kfd_alloc_process_doorbells(struct kfd_dev *kfd, struct kfd_process_device *
 	int r;
 	struct qcm_process_device *qpd = &pdd->qpd;

+	mutex_lock(&kfd->doorbell_mutex);
+
+	/* Another thread may have won the check-and-allocate race */
+	if (qpd->proc_doorbells) {
+		mutex_unlock(&kfd->doorbell_mutex);
+		return 0;
+	}
+
 	/* Allocate bitmap for dynamic doorbell allocation */
 	qpd->doorbell_bitmap = bitmap_zalloc(KFD_MAX_NUM_OF_QUEUES_PER_PROCESS,
 					     GFP_KERNEL);
 	if (!qpd->doorbell_bitmap) {
 		DRM_ERROR("Failed to allocate process doorbell bitmap\n");
-		return -ENOMEM;
+		r = -ENOMEM;
+		goto unlock;
 	}

 	r = init_doorbell_bitmap(&pdd->qpd, kfd);
@@ -284,11 +293,15 @@ int kfd_alloc_process_doorbells(struct kfd_dev *kfd, struct kfd_process_device *
 		DRM_ERROR("Failed to allocate process doorbells\n");
 		goto err;
 	}
+
+	mutex_unlock(&kfd->doorbell_mutex);
 	return 0;

 err:
 	bitmap_free(qpd->doorbell_bitmap);
 	qpd->doorbell_bitmap = NULL;
+unlock:
+	mutex_unlock(&kfd->doorbell_mutex);
 	return r;
 }

--
2.43.7

Re: [PATCH] amdkfd: fix doorbell allocation race creating duplicate allocations

[Kuehling, Felix]

On 2026-10-09 06:18, Henry Martin wrote:
> The check-and-allocate sequence on pdd->qpd.proc_doorbells is
> lockless at every call site of kfd_alloc_process_doorbells():
> kfd_get_process_doorbells() (mmap of /dev/kfd), kfd_ioctl_create_queue()
> and the CRIU restore path each test the pointer under different
> locking (or none), so two racing threads can both observe NULL and
> each allocate a doorbell BO and bitmap, orphaning one allocation and
> handing the two mappings different backing pages.
>
> Move the serialization into kfd_alloc_process_doorbells() itself:
> take kfd->doorbell_mutex and re-check under it, so all three call
> sites are covered regardless of their own locking.  The existing
> doorbell_mutex users (kfd_get_kernel_doorbell() /
> kfd_release_kernel_doorbell()) only guard the device-global
> kfd->doorbell_bitmap in short non-sleeping sections and never call
> into this path, so there is no nesting.
>
> This vulnerability was discovered by Tencent CodeBuddy Security.
>
> Fixes: 428542d91772 ("drm/amd/display: Setup for mmhubbub3_warmup_mcif with big buffer")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>

Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>

I'm applying the patch to amd-staging-drm-next.

Thanks,
   Felix


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> index fdcf7f2d1b5b4..07f881573837e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> @@ -257,12 +257,21 @@ int kfd_alloc_process_doorbells(struct kfd_dev *kfd, struct kfd_process_device *
>   	int r;
>   	struct qcm_process_device *qpd = &pdd->qpd;
>
> +	mutex_lock(&kfd->doorbell_mutex);
> +
> +	/* Another thread may have won the check-and-allocate race */
> +	if (qpd->proc_doorbells) {
> +		mutex_unlock(&kfd->doorbell_mutex);
> +		return 0;
> +	}
> +
>   	/* Allocate bitmap for dynamic doorbell allocation */
>   	qpd->doorbell_bitmap = bitmap_zalloc(KFD_MAX_NUM_OF_QUEUES_PER_PROCESS,
>   					     GFP_KERNEL);
>   	if (!qpd->doorbell_bitmap) {
>   		DRM_ERROR("Failed to allocate process doorbell bitmap\n");
> -		return -ENOMEM;
> +		r = -ENOMEM;
> +		goto unlock;
>   	}
>
>   	r = init_doorbell_bitmap(&pdd->qpd, kfd);
> @@ -284,11 +293,15 @@ int kfd_alloc_process_doorbells(struct kfd_dev *kfd, struct kfd_process_device *
>   		DRM_ERROR("Failed to allocate process doorbells\n");
>   		goto err;
>   	}
> +
> +	mutex_unlock(&kfd->doorbell_mutex);
>   	return 0;
>
>   err:
>   	bitmap_free(qpd->doorbell_bitmap);
>   	qpd->doorbell_bitmap = NULL;
> +unlock:
> +	mutex_unlock(&kfd->doorbell_mutex);
>   	return r;
>   }
>
> --
> 2.43.7

Re: [PATCH] amdkfd: fix doorbell allocation race creating duplicate allocations

[Kuehling, Felix]

On 2026-10-09 11:00, Kuehling, Felix wrote:
> On 2026-10-09 06:18, Henry Martin wrote:
>> The check-and-allocate sequence on pdd->qpd.proc_doorbells is
>> lockless at every call site of kfd_alloc_process_doorbells():
>> kfd_get_process_doorbells() (mmap of /dev/kfd), kfd_ioctl_create_queue()
>> and the CRIU restore path each test the pointer under different
>> locking (or none), so two racing threads can both observe NULL and
>> each allocate a doorbell BO and bitmap, orphaning one allocation and
>> handing the two mappings different backing pages.
>>
>> Move the serialization into kfd_alloc_process_doorbells() itself:
>> take kfd->doorbell_mutex and re-check under it, so all three call
>> sites are covered regardless of their own locking.  The existing
>> doorbell_mutex users (kfd_get_kernel_doorbell() /
>> kfd_release_kernel_doorbell()) only guard the device-global
>> kfd->doorbell_bitmap in short non-sleeping sections and never call
>> into this path, so there is no nesting.
>>
>> This vulnerability was discovered by Tencent CodeBuddy Security.
>>
>> Fixes: 428542d91772 ("drm/amd/display: Setup for 
>> mmhubbub3_warmup_mcif with big buffer")
>> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
>
> Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
>
> I'm applying the patch to amd-staging-drm-next.

An automatic Claude review pointed out two problems with the patch. I 
agree with both:

The Fixes: tag points at an unrelated drm/amd/display commit 
(428542d91772). It must be corrected — 2105a15a2046 ("drm/amdgpu: use 
doorbell mgr for kfd process doorbells") is the commit that introduced 
the allocation being fixed.

The patch holds the device-global kfd->doorbell_mutex across 
bitmap_zalloc(GFP_KERNEL) and amdgpu_bo_create_kernel(), while that same 
mutex is already acquired under dqm_lock via start_cpsch -> pm_init -> 
kq_initialize -> kfd_get_kernel_doorbell(). That is a plausible ABBA 
inversion once reclaim/TTM eviction runs the KFD eviction fence back 
into dqm_lock. Please either post lockdep (CONFIG_PROVE_LOCKING) results 
for a boot + queue-create cycle, or switch to a per-process-device lock, 
which also avoids serialising unrelated processes behind a device-wide 
mutex for per-process state.

Regards,
   Felix


>
> Thanks,
>   Felix
>
>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
>> index fdcf7f2d1b5b4..07f881573837e 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
>> @@ -257,12 +257,21 @@ int kfd_alloc_process_doorbells(struct kfd_dev 
>> *kfd, struct kfd_process_device *
>>       int r;
>>       struct qcm_process_device *qpd = &pdd->qpd;
>>
>> +    mutex_lock(&kfd->doorbell_mutex);
>> +
>> +    /* Another thread may have won the check-and-allocate race */
>> +    if (qpd->proc_doorbells) {
>> +        mutex_unlock(&kfd->doorbell_mutex);
>> +        return 0;
>> +    }
>> +
>>       /* Allocate bitmap for dynamic doorbell allocation */
>>       qpd->doorbell_bitmap = 
>> bitmap_zalloc(KFD_MAX_NUM_OF_QUEUES_PER_PROCESS,
>>                            GFP_KERNEL);
>>       if (!qpd->doorbell_bitmap) {
>>           DRM_ERROR("Failed to allocate process doorbell bitmap\n");
>> -        return -ENOMEM;
>> +        r = -ENOMEM;
>> +        goto unlock;
>>       }
>>
>>       r = init_doorbell_bitmap(&pdd->qpd, kfd);
>> @@ -284,11 +293,15 @@ int kfd_alloc_process_doorbells(struct kfd_dev 
>> *kfd, struct kfd_process_device *
>>           DRM_ERROR("Failed to allocate process doorbells\n");
>>           goto err;
>>       }
>> +
>> +    mutex_unlock(&kfd->doorbell_mutex);
>>       return 0;
>>
>>   err:
>>       bitmap_free(qpd->doorbell_bitmap);
>>       qpd->doorbell_bitmap = NULL;
>> +unlock:
>> +    mutex_unlock(&kfd->doorbell_mutex);
>>       return r;
>>   }
>>
>> -- 
>> 2.43.7

Re: [PATCH] amdkfd: fix doorbell allocation race creating duplicate allocations

[henry martin]

Hi Felix,

You're right on both counts, thanks for the careful look.

v2 drops kfd->doorbell_mutex entirely and adds a per-process-device
mutex in struct kfd_process_device (initialized in
kfd_create_process_device_data()).  The device-global mutex is no
longer held across the sleeping allocations, so the existing
dqm->lock -> doorbell_mutex order on the start_cpsch() -> pm_init()
-> kq_initialize() path cannot invert, and unrelated processes no
longer serialize against each other for per-process doorbell setup.

The Fixes tag is corrected to 2105a15a2046 ("drm/amdgpu: use
doorbell mgr for kfd process doorbells").

Regards,
Henry

Kuehling, Felix <felix.kuehling@amd.com> 于2026年10月9日周五 23:33写道：
>
> On 2026-10-09 11:00, Kuehling, Felix wrote:
> > On 2026-10-09 06:18, Henry Martin wrote:
> >> The check-and-allocate sequence on pdd->qpd.proc_doorbells is
> >> lockless at every call site of kfd_alloc_process_doorbells():
> >> kfd_get_process_doorbells() (mmap of /dev/kfd), kfd_ioctl_create_queue()
> >> and the CRIU restore path each test the pointer under different
> >> locking (or none), so two racing threads can both observe NULL and
> >> each allocate a doorbell BO and bitmap, orphaning one allocation and
> >> handing the two mappings different backing pages.
> >>
> >> Move the serialization into kfd_alloc_process_doorbells() itself:
> >> take kfd->doorbell_mutex and re-check under it, so all three call
> >> sites are covered regardless of their own locking.  The existing
> >> doorbell_mutex users (kfd_get_kernel_doorbell() /
> >> kfd_release_kernel_doorbell()) only guard the device-global
> >> kfd->doorbell_bitmap in short non-sleeping sections and never call
> >> into this path, so there is no nesting.
> >>
> >> This vulnerability was discovered by Tencent CodeBuddy Security.
> >>
> >> Fixes: 428542d91772 ("drm/amd/display: Setup for
> >> mmhubbub3_warmup_mcif with big buffer")
> >> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
> >
> > Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
> >
> > I'm applying the patch to amd-staging-drm-next.
>
> An automatic Claude review pointed out two problems with the patch. I
> agree with both:
>
> The Fixes: tag points at an unrelated drm/amd/display commit
> (428542d91772). It must be corrected — 2105a15a2046 ("drm/amdgpu: use
> doorbell mgr for kfd process doorbells") is the commit that introduced
> the allocation being fixed.
>
> The patch holds the device-global kfd->doorbell_mutex across
> bitmap_zalloc(GFP_KERNEL) and amdgpu_bo_create_kernel(), while that same
> mutex is already acquired under dqm_lock via start_cpsch -> pm_init ->
> kq_initialize -> kfd_get_kernel_doorbell(). That is a plausible ABBA
> inversion once reclaim/TTM eviction runs the KFD eviction fence back
> into dqm_lock. Please either post lockdep (CONFIG_PROVE_LOCKING) results
> for a boot + queue-create cycle, or switch to a per-process-device lock,
> which also avoids serialising unrelated processes behind a device-wide
> mutex for per-process state.
>
> Regards,
>    Felix
>
>
> >
> > Thanks,
> >   Felix
> >
> >
> >> ---
> >>   drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 15 ++++++++++++++-
> >>   1 file changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> >> b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> >> index fdcf7f2d1b5b4..07f881573837e 100644
> >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> >> @@ -257,12 +257,21 @@ int kfd_alloc_process_doorbells(struct kfd_dev
> >> *kfd, struct kfd_process_device *
> >>       int r;
> >>       struct qcm_process_device *qpd = &pdd->qpd;
> >>
> >> +    mutex_lock(&kfd->doorbell_mutex);
> >> +
> >> +    /* Another thread may have won the check-and-allocate race */
> >> +    if (qpd->proc_doorbells) {
> >> +        mutex_unlock(&kfd->doorbell_mutex);
> >> +        return 0;
> >> +    }
> >> +
> >>       /* Allocate bitmap for dynamic doorbell allocation */
> >>       qpd->doorbell_bitmap =
> >> bitmap_zalloc(KFD_MAX_NUM_OF_QUEUES_PER_PROCESS,
> >>                            GFP_KERNEL);
> >>       if (!qpd->doorbell_bitmap) {
> >>           DRM_ERROR("Failed to allocate process doorbell bitmap\n");
> >> -        return -ENOMEM;
> >> +        r = -ENOMEM;
> >> +        goto unlock;
> >>       }
> >>
> >>       r = init_doorbell_bitmap(&pdd->qpd, kfd);
> >> @@ -284,11 +293,15 @@ int kfd_alloc_process_doorbells(struct kfd_dev
> >> *kfd, struct kfd_process_device *
> >>           DRM_ERROR("Failed to allocate process doorbells\n");
> >>           goto err;
> >>       }
> >> +
> >> +    mutex_unlock(&kfd->doorbell_mutex);
> >>       return 0;
> >>
> >>   err:
> >>       bitmap_free(qpd->doorbell_bitmap);
> >>       qpd->doorbell_bitmap = NULL;
> >> +unlock:
> >> +    mutex_unlock(&kfd->doorbell_mutex);
> >>       return r;
> >>   }
> >>
> >> --
> >> 2.43.7

[PATCH v2] amdkfd: fix doorbell allocation race creating duplicate allocations

[Henry Martin]

The check-and-allocate sequence on pdd->qpd.proc_doorbells is
lockless at every call site of kfd_alloc_process_doorbells():
kfd_get_process_doorbells() (mmap of /dev/kfd),
kfd_ioctl_create_queue() and the CRIU restore path each test the
pointer under different locking (or none), so two racing threads can
both observe NULL and each allocate a doorbell BO and bitmap,
orphaning one allocation and handing the two mappings different
backing pages.

Move the serialization into kfd_alloc_process_doorbells() itself,
using a new per-process-device mutex so all three call sites are
covered regardless of their own locking.  The device-global
kfd->doorbell_mutex is not suitable here: it is acquired under
dqm->lock on the kernel-doorbell path (start_cpsch() -> pm_init() ->
kq_initialize() -> kfd_get_kernel_doorbell()), so holding it across
the sleeping allocations above would create a lock-ordering
inversion with dqm->lock via TTM eviction, and it would also
serialize doorbell setup of unrelated processes behind a
device-wide lock for per-process state.

This issue was discovered by Tencent CodeBuddy Security.

Cc: stable@vger.kernel.org
Fixes: 2105a15a2046 ("drm/amdgpu: use doorbell mgr for kfd process doorbells")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---

v2: Use a per-process-device mutex instead of kfd->doorbell_mutex
    (avoids the dqm->lock ordering inversion and the device-wide
    serialization) and correct the Fixes tag.  Pointed out by Felix
    Kuehling.

 drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 15 ++++++++++++++-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h     |  3 +++
 drivers/gpu/drm/amd/amdkfd/kfd_process.c  |  1 +
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
index fdcf7f2d1b5b4..55acadc986bee 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
@@ -257,12 +257,21 @@ int kfd_alloc_process_doorbells(struct kfd_dev *kfd, struct kfd_process_device *
 	int r;
 	struct qcm_process_device *qpd = &pdd->qpd;
 
+	mutex_lock(&pdd->doorbell_mutex);
+
+	/* Another thread may have won the check-and-allocate race */
+	if (qpd->proc_doorbells) {
+		mutex_unlock(&pdd->doorbell_mutex);
+		return 0;
+	}
+
 	/* Allocate bitmap for dynamic doorbell allocation */
 	qpd->doorbell_bitmap = bitmap_zalloc(KFD_MAX_NUM_OF_QUEUES_PER_PROCESS,
 					     GFP_KERNEL);
 	if (!qpd->doorbell_bitmap) {
 		DRM_ERROR("Failed to allocate process doorbell bitmap\n");
-		return -ENOMEM;
+		r = -ENOMEM;
+		goto unlock;
 	}
 
 	r = init_doorbell_bitmap(&pdd->qpd, kfd);
@@ -284,11 +293,15 @@ int kfd_alloc_process_doorbells(struct kfd_dev *kfd, struct kfd_process_device *
 		DRM_ERROR("Failed to allocate process doorbells\n");
 		goto err;
 	}
+
+	mutex_unlock(&pdd->doorbell_mutex);
 	return 0;
 
 err:
 	bitmap_free(qpd->doorbell_bitmap);
 	qpd->doorbell_bitmap = NULL;
+unlock:
+	mutex_unlock(&pdd->doorbell_mutex);
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index d8631847f0eb9..96e59133fee02 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -779,6 +779,9 @@ struct kfd_process_device {
 	/* per-process-per device QCM data structure */
 	struct qcm_process_device qpd;
 
+	/* Serializes process doorbell allocation for this device */
+	struct mutex doorbell_mutex;
+
 	/*Apertures*/
 	uint64_t lds_base;
 	uint64_t lds_limit;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 0a7c1900da959..4efe4dae79c13 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1788,6 +1788,7 @@ struct kfd_process_device *kfd_create_process_device_data(struct kfd_node *dev,
 	pdd->dev = dev;
 	INIT_LIST_HEAD(&pdd->qpd.queues_list);
 	INIT_LIST_HEAD(&pdd->qpd.priv_queue_list);
+	mutex_init(&pdd->doorbell_mutex);
 	pdd->qpd.dqm = dev->dqm;
 	pdd->qpd.pqm = &p->pqm;
 	pdd->qpd.evicted = 0;
-- 
2.43.7