[PATCH] drm/amdkfd: use kzalloc_objs() instead of kzalloc() with multiply

[PATCH] drm/amdkfd: use kzalloc_objs() instead of kzalloc() with multiply

[Hrushiraj Gandhi]

dqm->detect_hang_info, dqm->hung_db_array and dqm->hqd_info are all
allocated with a hand-computed count * sizeof(TYPE) size. If count
were ever attacker- or firmware-influenced and large enough, the
multiply could wrap size_t and produce a too-small allocation that
the code then writes count-many elements into.

kzalloc_objs() computes the same size via size_mul(), which saturates
to SIZE_MAX on overflow instead of wrapping, so the allocation itself
fails cleanly (already handled here via the existing NULL checks)
rather than silently succeeding undersized.

dqm->detect_hang_info_size is left as-is since it is also used later
for a memset() call; only the allocation itself is converted.

No functional change in the non-overflowing case.

Signed-off-by: Hrushiraj Gandhi <hrushirajg23@gmail.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 67137679a901..5aa42fd5bb49 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -2013,17 +2013,15 @@ static int start_cpsch(struct device_queue_manager *dqm)
 			      NUM_XCC(dqm->dev->xcc_mask);
 
 	dqm->detect_hang_info_size = num_hw_queue_slots * sizeof(struct dqm_detect_hang_info);
-	dqm->detect_hang_info = kzalloc(dqm->detect_hang_info_size, GFP_KERNEL);
+	dqm->detect_hang_info = kzalloc_objs(*dqm->detect_hang_info, num_hw_queue_slots);
 
 	if (!dqm->detect_hang_info) {
 		retval = -ENOMEM;
 		goto fail_detect_hang_buffer;
 	}
 
-	dqm->hung_db_array = kzalloc(hung_array_size * sizeof(u32), GFP_KERNEL);
-	dqm->hqd_info = kzalloc(
-		hqd_info_size * sizeof(struct amdgpu_mes_hung_queue_hqd_info),
-		GFP_KERNEL);
+	dqm->hung_db_array = kzalloc_objs(*dqm->hung_db_array, hung_array_size);
+	dqm->hqd_info = kzalloc_objs(*dqm->hqd_info, hqd_info_size);
 
 	dqm_unlock(dqm);
 

Re: [PATCH] drm/amdkfd: use kzalloc_objs() instead of kzalloc() with multiply

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Double free of `dqm->fence_mem`, `dqm->hung_db_array`, `dqm->hqd_info` and packet manager due to `dqm->sched_running` remaining true on `start_cpsch` error path.
--

commit 9326059d02e56cf390501c700d0f8e0e83592163
Author: Hrushiraj Gandhi <hrushirajg23@gmail.com>

    drm/amdkfd: use kzalloc_objs() instead of kzalloc() with multiply
    
    dqm->detect_hang_info, dqm->hung_db_array and dqm->hqd_info are all
    allocated with a hand-computed count * sizeof(TYPE) size. If count
    were ever attacker- or firmware-influenced and large enough, the
    multiply could wrap size_t and produce a too-small allocation that
    the code then writes count-many elements into.
    
    kzalloc_objs() computes the same size via size_mul(), which saturates
    to SIZE_MAX on overflow instead of wrapping, so the allocation itself
    fails cleanly (already handled here via the existing NULL checks)
    rather than silently succeeding undersized.
    
    dqm->detect_hang_info_size is left as-is since it is also used later
    for a memset() call; only the allocation itself is converted.
    
    No functional change in the non-overflowing case.
    
    Signed-off-by: Hrushiraj Gandhi <hrushirajg23@gmail.com>

> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 69cd4441e240c..488176e947c3e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -2160,17 +2160,15 @@ static int start_cpsch(struct device_queue_manager *dqm)
>  			      NUM_XCC(dqm->dev->xcc_mask);
>  
>  	dqm->detect_hang_info_size = num_hw_queue_slots * sizeof(struct dqm_detect_hang_info);
> -	dqm->detect_hang_info = kzalloc(dqm->detect_hang_info_size, GFP_KERNEL);
> +	dqm->detect_hang_info = kzalloc_objs(*dqm->detect_hang_info, num_hw_queue_slots);
>  
>  	if (!dqm->detect_hang_info) {
>  		retval = -ENOMEM;
>  		goto fail_detect_hang_buffer;

[Severity: High]
This isn't a bug introduced by this patch, but does this error path leave
dqm->sched_running set to true?

Earlier in start_cpsch(), dqm->sched_running is set to true. When this
allocation fails, the error path executes kfd_gtt_sa_free() on dqm->fence_mem
and uninitializes the packet manager:

fail_detect_hang_buffer:
	kfd_gtt_sa_free(dqm->dev, dqm->fence_mem);
fail_allocate_vidmem:
fail_set_sched_resources:
	if (!dqm->dev->kfd->shared_resources.enable_mes)
		pm_uninit(&dqm->packet_mgr);

Because it fails to clear dqm->sched_running or set dqm->fence_mem to NULL,
subsequent cleanup via stop_cpsch() will proceed rather than exiting early.
This causes stop_cpsch() to call kfd_gtt_sa_free() on the dangling fence_mem
pointer and pm_uninit() a second time. The second pm_uninit() call passes a
NULL pointer which gets dereferenced, leading to a kernel panic.

Additionally, stop_cpsch() frees dqm->hung_db_array and dqm->hqd_info without
nullifying them, meaning a subsequent start/stop cycle could trigger a double
free on those arrays as well.

Could dqm->sched_running be set to false and the freed pointers cleared to
NULL in this error path to avoid the double free?

>  	}
>  
> -	dqm->hung_db_array = kzalloc(hung_array_size * sizeof(u32), GFP_KERNEL);
> -	dqm->hqd_info = kzalloc(
> -		hqd_info_size * sizeof(struct amdgpu_mes_hung_queue_hqd_info),
> -		GFP_KERNEL);
> +	dqm->hung_db_array = kzalloc_objs(*dqm->hung_db_array, hung_array_size);
> +	dqm->hqd_info = kzalloc_objs(*dqm->hqd_info, hqd_info_size);
>  
>  	dqm_unlock(dqm);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901072615.522925-1-hrushirajg23@gmail.com?part=1

Re: [PATCH] drm/amdkfd: use kzalloc_objs() instead of kzalloc() with multiply

[Alex Deucher]

On Tue, Sep 1, 2026 at 3:59 AM Hrushiraj Gandhi <hrushirajg23@gmail.com> wrote:
>
> dqm->detect_hang_info, dqm->hung_db_array and dqm->hqd_info are all
> allocated with a hand-computed count * sizeof(TYPE) size. If count
> were ever attacker- or firmware-influenced and large enough, the
> multiply could wrap size_t and produce a too-small allocation that
> the code then writes count-many elements into.
>
> kzalloc_objs() computes the same size via size_mul(), which saturates
> to SIZE_MAX on overflow instead of wrapping, so the allocation itself
> fails cleanly (already handled here via the existing NULL checks)
> rather than silently succeeding undersized.
>
> dqm->detect_hang_info_size is left as-is since it is also used later
> for a memset() call; only the allocation itself is converted.
>
> No functional change in the non-overflowing case.
>
> Signed-off-by: Hrushiraj Gandhi <hrushirajg23@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 67137679a901..5aa42fd5bb49 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -2013,17 +2013,15 @@ static int start_cpsch(struct device_queue_manager *dqm)
>                               NUM_XCC(dqm->dev->xcc_mask);
>
>         dqm->detect_hang_info_size = num_hw_queue_slots * sizeof(struct dqm_detect_hang_info);

Can you fix this calculation to check for an overflow while you are at it?

Alex

> -       dqm->detect_hang_info = kzalloc(dqm->detect_hang_info_size, GFP_KERNEL);
> +       dqm->detect_hang_info = kzalloc_objs(*dqm->detect_hang_info, num_hw_queue_slots);
>
>         if (!dqm->detect_hang_info) {
>                 retval = -ENOMEM;
>                 goto fail_detect_hang_buffer;
>         }
>
> -       dqm->hung_db_array = kzalloc(hung_array_size * sizeof(u32), GFP_KERNEL);
> -       dqm->hqd_info = kzalloc(
> -               hqd_info_size * sizeof(struct amdgpu_mes_hung_queue_hqd_info),
> -               GFP_KERNEL);
> +       dqm->hung_db_array = kzalloc_objs(*dqm->hung_db_array, hung_array_size);
> +       dqm->hqd_info = kzalloc_objs(*dqm->hqd_info, hqd_info_size);
>
>         dqm_unlock(dqm);
>