[PATCH] drm/vkms: Adjust vkms_state->active_planes allocation type

[PATCH] drm/vkms: Adjust vkms_state->active_planes allocation type

[Kees Cook]

In preparation for making the kmalloc family of allocators type aware,
we need to make sure that the returned type from the allocation matches
the type of the variable being assigned. (Before, the allocator would
always return "void *", which can be implicitly cast to any pointer type.)

The assigned type is "struct vkms_plane_state **", but the returned type
will be "struct drm_plane **". These are the same size (pointer size), but
the types don't match. Adjust the allocation type to match the assignment.

Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Louis Chauvet <louis.chauvet@bootlin.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Melissa Wen <melissa.srw@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: <dri-devel@lists.freedesktop.org>
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 12034ec12029..8c9898b9055d 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -194,7 +194,7 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
 		i++;
 	}
 
-	vkms_state->active_planes = kcalloc(i, sizeof(plane), GFP_KERNEL);
+	vkms_state->active_planes = kcalloc(i, sizeof(*vkms_state->active_planes), GFP_KERNEL);
 	if (!vkms_state->active_planes)
 		return -ENOMEM;
 	vkms_state->num_active_planes = i;
-- 
2.34.1

Re: [PATCH] drm/vkms: Adjust vkms_state->active_planes allocation type

[Louis Chauvet]

Le 26/04/2025 à 08:14, Kees Cook a écrit :
> In preparation for making the kmalloc family of allocators type aware,
> we need to make sure that the returned type from the allocation matches
> the type of the variable being assigned. (Before, the allocator would
> always return "void *", which can be implicitly cast to any pointer type.)
> 
> The assigned type is "struct vkms_plane_state **", but the returned type
> will be "struct drm_plane **". These are the same size (pointer size), but
> the types don't match. Adjust the allocation type to match the assignment.

I think this is an issue, can you add the proper Fixup tag in this commit?

With this:

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> Cc: Louis Chauvet <louis.chauvet@bootlin.com>
> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Melissa Wen <melissa.srw@gmail.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: <dri-devel@lists.freedesktop.org>
> ---
>   drivers/gpu/drm/vkms/vkms_crtc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 12034ec12029..8c9898b9055d 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -194,7 +194,7 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
>   		i++;
>   	}
>   
> -	vkms_state->active_planes = kcalloc(i, sizeof(plane), GFP_KERNEL);
> +	vkms_state->active_planes = kcalloc(i, sizeof(*vkms_state->active_planes), GFP_KERNEL);
>   	if (!vkms_state->active_planes)
>   		return -ENOMEM;
>   	vkms_state->num_active_planes = i;

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] drm/vkms: Adjust vkms_state->active_planes allocation type

[Kees Cook]

On Mon, Apr 28, 2025 at 10:18:34AM +0200, Louis Chauvet wrote:
> 
> 
> Le 26/04/2025 à 08:14, Kees Cook a écrit :
> > In preparation for making the kmalloc family of allocators type aware,
> > we need to make sure that the returned type from the allocation matches
> > the type of the variable being assigned. (Before, the allocator would
> > always return "void *", which can be implicitly cast to any pointer type.)
> > 
> > The assigned type is "struct vkms_plane_state **", but the returned type
> > will be "struct drm_plane **". These are the same size (pointer size), but
> > the types don't match. Adjust the allocation type to match the assignment.
> 
> I think this is an issue, can you add the proper Fixup tag in this commit?

I think trailers updating tools like b4 will pick this up:

Fixes: 8b1865873651 ("drm/vkms: totally reworked crc data tracking")

Would you rather I send a v2 with the Fixes added?

> 
> With this:
> 
> Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

Thanks!

-Kees

-- 
Kees Cook

Re: [PATCH] drm/vkms: Adjust vkms_state->active_planes allocation type

[Louis Chauvet]

On Fri, 25 Apr 2025 23:14:32 -0700, Kees Cook wrote:
> In preparation for making the kmalloc family of allocators type aware,
> we need to make sure that the returned type from the allocation matches
> the type of the variable being assigned. (Before, the allocator would
> always return "void *", which can be implicitly cast to any pointer type.)
> 
> The assigned type is "struct vkms_plane_state **", but the returned type
> will be "struct drm_plane **". These are the same size (pointer size), but
> the types don't match. Adjust the allocation type to match the assignment.
> 
> [...]

Applied, thanks!

[1/1] drm/vkms: Adjust vkms_state->active_planes allocation type
      commit: 258aebf100540d36aba910f545d4d5ddf4ecaf0b

Best regards,
-- 
Louis Chauvet <contact@louischauvet.fr>