drm/amd/display: Use kmalloc_array() instead of kmalloc()

[PATCH] drm/amd/display: Use kmalloc_array() instead of kmalloc()

Posted by James Flowers 2 weeks, 5 days ago

Documentation/process/deprecated.rst recommends against the use of kmalloc
with dynamic size calculations due to the risk of overflow and smaller
allocation being made than the caller was expecting. This could lead to
buffer overflow in code similar to the memcpy in
amdgpu_dm_plane_add_modifier().

Signed-off-by: James Flowers <bold.zone2373@fastmail.com>
---
I see that in amdgpu_dm_plane_get_plane_modifiers, capacity is initialized to 
only 128, but it is probably preferable to refactor.
 
Tested on a Steam Deck OLED with no apparent regressions using these test suites from
igt-gpu-tools: 
1) kms_plane
2) amd_plane
3) amd_fuzzing
4) testdisplay 

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index b7c6e8d13435..b587d2033f0b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -146,7 +146,7 @@ static void amdgpu_dm_plane_add_modifier(uint64_t **mods, uint64_t *size, uint64
 
 	if (*cap - *size < 1) {
 		uint64_t new_cap = *cap * 2;
-		uint64_t *new_mods = kmalloc(new_cap * sizeof(uint64_t), GFP_KERNEL);
+		uint64_t *new_mods = kmalloc_array(new_cap, sizeof(uint64_t), GFP_KERNEL);
 
 		if (!new_mods) {
 			kfree(*mods);
@@ -732,7 +732,7 @@ static int amdgpu_dm_plane_get_plane_modifiers(struct amdgpu_device *adev, unsig
 	if (adev->family < AMDGPU_FAMILY_AI)
 		return 0;
 
-	*mods = kmalloc(capacity * sizeof(uint64_t), GFP_KERNEL);
+	*mods = kmalloc_array(capacity, sizeof(uint64_t), GFP_KERNEL);
 
 	if (plane_type == DRM_PLANE_TYPE_CURSOR) {
 		amdgpu_dm_plane_add_modifier(mods, &size, &capacity, DRM_FORMAT_MOD_LINEAR);
-- 
2.51.0

Re: [PATCH] drm/amd/display: Use kmalloc_array() instead of kmalloc()

Posted by Alex Hung 2 weeks, 2 days ago

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 9/12/25 23:19, James Flowers wrote:
> Documentation/process/deprecated.rst recommends against the use of kmalloc
> with dynamic size calculations due to the risk of overflow and smaller
> allocation being made than the caller was expecting. This could lead to
> buffer overflow in code similar to the memcpy in
> amdgpu_dm_plane_add_modifier().
> 
> Signed-off-by: James Flowers <bold.zone2373@fastmail.com>
> ---
> I see that in amdgpu_dm_plane_get_plane_modifiers, capacity is initialized to
> only 128, but it is probably preferable to refactor.
>   
> Tested on a Steam Deck OLED with no apparent regressions using these test suites from
> igt-gpu-tools:
> 1) kms_plane
> 2) amd_plane
> 3) amd_fuzzing
> 4) testdisplay
> 
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index b7c6e8d13435..b587d2033f0b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -146,7 +146,7 @@ static void amdgpu_dm_plane_add_modifier(uint64_t **mods, uint64_t *size, uint64
>   
>   	if (*cap - *size < 1) {
>   		uint64_t new_cap = *cap * 2;
> -		uint64_t *new_mods = kmalloc(new_cap * sizeof(uint64_t), GFP_KERNEL);
> +		uint64_t *new_mods = kmalloc_array(new_cap, sizeof(uint64_t), GFP_KERNEL);
>   
>   		if (!new_mods) {
>   			kfree(*mods);
> @@ -732,7 +732,7 @@ static int amdgpu_dm_plane_get_plane_modifiers(struct amdgpu_device *adev, unsig
>   	if (adev->family < AMDGPU_FAMILY_AI)
>   		return 0;
>   
> -	*mods = kmalloc(capacity * sizeof(uint64_t), GFP_KERNEL);
> +	*mods = kmalloc_array(capacity, sizeof(uint64_t), GFP_KERNEL);
>   
>   	if (plane_type == DRM_PLANE_TYPE_CURSOR) {
>   		amdgpu_dm_plane_add_modifier(mods, &size, &capacity, DRM_FORMAT_MOD_LINEAR);

Re: [PATCH] drm/amd/display: Use kmalloc_array() instead of kmalloc()

Posted by Alex Deucher 2 weeks, 2 days ago

Applied.  Thanks!

Alex

On Sat, Sep 13, 2025 at 1:31 AM James Flowers
<bold.zone2373@fastmail.com> wrote:
>
> Documentation/process/deprecated.rst recommends against the use of kmalloc
> with dynamic size calculations due to the risk of overflow and smaller
> allocation being made than the caller was expecting. This could lead to
> buffer overflow in code similar to the memcpy in
> amdgpu_dm_plane_add_modifier().
>
> Signed-off-by: James Flowers <bold.zone2373@fastmail.com>
> ---
> I see that in amdgpu_dm_plane_get_plane_modifiers, capacity is initialized to
> only 128, but it is probably preferable to refactor.
>
> Tested on a Steam Deck OLED with no apparent regressions using these test suites from
> igt-gpu-tools:
> 1) kms_plane
> 2) amd_plane
> 3) amd_fuzzing
> 4) testdisplay
>
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index b7c6e8d13435..b587d2033f0b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -146,7 +146,7 @@ static void amdgpu_dm_plane_add_modifier(uint64_t **mods, uint64_t *size, uint64
>
>         if (*cap - *size < 1) {
>                 uint64_t new_cap = *cap * 2;
> -               uint64_t *new_mods = kmalloc(new_cap * sizeof(uint64_t), GFP_KERNEL);
> +               uint64_t *new_mods = kmalloc_array(new_cap, sizeof(uint64_t), GFP_KERNEL);
>
>                 if (!new_mods) {
>                         kfree(*mods);
> @@ -732,7 +732,7 @@ static int amdgpu_dm_plane_get_plane_modifiers(struct amdgpu_device *adev, unsig
>         if (adev->family < AMDGPU_FAMILY_AI)
>                 return 0;
>
> -       *mods = kmalloc(capacity * sizeof(uint64_t), GFP_KERNEL);
> +       *mods = kmalloc_array(capacity, sizeof(uint64_t), GFP_KERNEL);
>
>         if (plane_type == DRM_PLANE_TYPE_CURSOR) {
>                 amdgpu_dm_plane_add_modifier(mods, &size, &capacity, DRM_FORMAT_MOD_LINEAR);
> --
> 2.51.0
>