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

Hrushiraj Gandhi posted 1 patch 3 weeks, 4 days ago
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
[PATCH] drm/amdkfd: use kzalloc_objs() instead of kzalloc() with multiply
Posted by Hrushiraj Gandhi 3 weeks, 4 days ago
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
Posted by Alex Deucher 3 weeks, 3 days ago
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);
>