* Re: [PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()
2026-04-09 16:41 [PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs() Ashutosh Desai
@ 2026-04-10 8:26 ` Jani Nikula
2026-04-10 22:10 ` Ashutosh Desai
2026-04-17 9:02 ` Thomas Zimmermann
2 siblings, 0 replies; 5+ messages in thread
From: Jani Nikula @ 2026-04-10 8:26 UTC (permalink / raw)
To: Ashutosh Desai, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, Ashutosh Desai
On Thu, 09 Apr 2026, Ashutosh Desai <ashutoshdesai993@gmail.com> wrote:
> drm_gem_fb_init_with_funcs() computes sub-sampled plane dimensions
> using plain integer division:
>
> unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
>
> However, the ioctl-level framebuffer_check() in drm_framebuffer.c uses
> drm_format_info_plane_width/height() which round up dimensions via
> DIV_ROUND_UP(). This inconsistency corrupts the subsequent GEM object
> size check for certain pixel format and dimension combinations.
>
> For example, with NV12 (vsub=2) and a 1-pixel-tall framebuffer the
> GEM size validation path sees height=0 instead of height=1. The
> expression (height - 1) then wraps to UINT_MAX as an unsigned int,
> causing min_size to overflow and wrap back to a small value. A tiny
> GEM object therefore passes the size guard, yet when the GPU accesses
> the chroma plane it will read or write memory beyond the object's
> bounds.
>
> Fix by replacing the open-coded divisions with drm_format_info_plane_width()
> and drm_format_info_plane_height(), which use DIV_ROUND_UP() and match
> the calculation already used in framebuffer_check().
>
> Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
Hello Ashutosh -
Receiving a handful of highly polished patches in quick succession on
various parts of the drm subsystem from someone who has no commits in
the kernel and has no previous interactions on the mailing lists is
virtually unheard of.
I have to ask, did you use AI coding assistants? Please read the kernel
documentation on AI coding assistants and attribution [1].
BR,
Jani.
[1] https://docs.kernel.org/process/coding-assistants.html
> ---
> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 9166c353f..88808e972 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -172,8 +172,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
> }
>
> for (i = 0; i < info->num_planes; i++) {
> - unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> - unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> + unsigned int width = drm_format_info_plane_width(info, mode_cmd->width, i);
> + unsigned int height = drm_format_info_plane_height(info, mode_cmd->height, i);
> unsigned int min_size;
>
> objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()
2026-04-09 16:41 [PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs() Ashutosh Desai
2026-04-10 8:26 ` Jani Nikula
@ 2026-04-10 22:10 ` Ashutosh Desai
2026-04-17 9:02 ` Thomas Zimmermann
2 siblings, 0 replies; 5+ messages in thread
From: Ashutosh Desai @ 2026-04-10 22:10 UTC (permalink / raw)
To: Jani Nikula
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, dri-devel, linux-kernel
Hi Jani,
Thank you for your question. It is a valid one. I have used AI to a small extent to help me understand the subsystem because it was something new for me. I haven't used AI to identify any bugs or create patches. This type of bug is something that I am always on the lookout for (size checking and boundary checks). So, these became evident during my review. Since AI wasn't used for code generation, I didn't think attribution was required, but please let me know if you'd prefer otherwise.
Would be happy to walk through any of the patches if it is helpful.
Best regards,
Ashutosh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()
2026-04-09 16:41 [PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs() Ashutosh Desai
2026-04-10 8:26 ` Jani Nikula
2026-04-10 22:10 ` Ashutosh Desai
@ 2026-04-17 9:02 ` Thomas Zimmermann
2026-04-20 1:36 ` [PATCH v2] " Ashutosh Desai
2 siblings, 1 reply; 5+ messages in thread
From: Thomas Zimmermann @ 2026-04-17 9:02 UTC (permalink / raw)
To: Ashutosh Desai, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: dri-devel, linux-kernel
Am 09.04.26 um 18:41 schrieb Ashutosh Desai:
> drm_gem_fb_init_with_funcs() computes sub-sampled plane dimensions
> using plain integer division:
>
> unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
>
> However, the ioctl-level framebuffer_check() in drm_framebuffer.c uses
> drm_format_info_plane_width/height() which round up dimensions via
> DIV_ROUND_UP(). This inconsistency corrupts the subsequent GEM object
> size check for certain pixel format and dimension combinations.
>
> For example, with NV12 (vsub=2) and a 1-pixel-tall framebuffer the
> GEM size validation path sees height=0 instead of height=1. The
> expression (height - 1) then wraps to UINT_MAX as an unsigned int,
> causing min_size to overflow and wrap back to a small value. A tiny
> GEM object therefore passes the size guard, yet when the GPU accesses
> the chroma plane it will read or write memory beyond the object's
> bounds.
>
> Fix by replacing the open-coded divisions with drm_format_info_plane_width()
> and drm_format_info_plane_height(), which use DIV_ROUND_UP() and match
> the calculation already used in framebuffer_check().
>
> Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
Thanks for the fix.
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 4c3dbb2c312c ("drm: Add GEM backed framebuffer library")
Cc: <stable@vger.kernel.org> # v4.14+
> ---
> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 9166c353f..88808e972 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -172,8 +172,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
> }
>
> for (i = 0; i < info->num_planes; i++) {
> - unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> - unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> + unsigned int width = drm_format_info_plane_width(info, mode_cmd->width, i);
> + unsigned int height = drm_format_info_plane_height(info, mode_cmd->height, i);
> unsigned int min_size;
>
> objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()
2026-04-17 9:02 ` Thomas Zimmermann
@ 2026-04-20 1:36 ` Ashutosh Desai
0 siblings, 0 replies; 5+ messages in thread
From: Ashutosh Desai @ 2026-04-20 1:36 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, simona
Cc: dri-devel, linux-kernel, stable, Ashutosh Desai
drm_gem_fb_init_with_funcs() computes sub-sampled plane dimensions
using plain integer division:
unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
However, the ioctl-level framebuffer_check() in drm_framebuffer.c uses
drm_format_info_plane_width/height() which round up dimensions via
DIV_ROUND_UP(). This inconsistency corrupts the subsequent GEM object
size check for certain pixel format and dimension combinations.
For example, with NV12 (vsub=2) and a 1-pixel-tall framebuffer the
GEM size validation path sees height=0 instead of height=1. The
expression (height - 1) then wraps to UINT_MAX as an unsigned int,
causing min_size to overflow and wrap back to a small value. A tiny
GEM object therefore passes the size guard, yet when the GPU accesses
the chroma plane it will read or write memory beyond the object's
bounds.
Fix by replacing the open-coded divisions with drm_format_info_plane_width()
and drm_format_info_plane_height(), which use DIV_ROUND_UP() and match
the calculation already used in framebuffer_check().
Fixes: 4c3dbb2c312c ("drm: Add GEM backed framebuffer library")
Cc: stable@vger.kernel.org # v4.14+
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
V1 -> V2: add Fixes: tag, Cc: stable@vger.kernel.org, incorporate
Reviewed-by from Thomas Zimmermann.
Link: https://lore.kernel.org/dri-devel/20260409164156.2235189-1-ashutoshdesai993@gmail.com/
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 9166c353f131..88808e972cc1 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -172,8 +172,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
}
for (i = 0; i < info->num_planes; i++) {
- unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
- unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
+ unsigned int width = drm_format_info_plane_width(info, mode_cmd->width, i);
+ unsigned int height = drm_format_info_plane_height(info, mode_cmd->height, i);
unsigned int min_size;
objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread