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@xxxxxxxxx>

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

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