[PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()

[PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()

[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>
---
 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]);
-- 
2.34.1

Re: [PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()

[Jani Nikula]

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

Re: [PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()

[ashutosh desai]

[-- Attachment #1: Type: text/plain, Size: 3650 bytes --]

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

On Fri, Apr 10, 2026 at 1:26 AM Jani Nikula <jani.nikula@linux.intel.com>
wrote:

> 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
>

[-- Attachment #2: Type: text/html, Size: 4584 bytes --]

Re: [PATCH] drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()

[Ashutosh Desai]

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

Claude review: drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()

[Claude Code Review Bot]

Overall Series Review

Subject: drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()
Author: Ashutosh Desai <ashutoshdesai993@gmail.com>
Patches: 4
Reviewed: 2026-04-12T10:54:06.081036

---

This is a single-patch fix for a real inconsistency between `framebuffer_check()` in `drm_framebuffer.c` and `drm_gem_fb_init_with_funcs()` in `drm_gem_framebuffer_helper.c`. The analysis in the commit message is correct: the open-coded truncating division can produce `height=0` for chroma planes when the framebuffer height is not evenly divisible by `vsub` (and likewise for width/hsub). The `(height - 1)` expression at line 186 then wraps as an unsigned integer, corrupting the `min_size` check.

The fix is straightforward, correct, and aligns the GEM size validation with the dimension calculation already used by `framebuffer_check()`. **The patch should be accepted**, but it has process issues that should be addressed before merging.

**Verdict: Needs minor revision (process tags), code change itself is correct.**

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/gem: Fix inconsistent plane dimension calculation in drm_gem_fb_init_with_funcs()

[Claude Code Review Bot]

Patch Review

**Code correctness: Good.** The change from:
```c
unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
```
to:
```c
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);
```

is correct. Looking at the helper definitions in `include/drm/drm_fourcc.h:272-305`, for `plane == 0` they return the original value (matching the old `(i ? ... : 1)` logic), and for `plane > 0` they use `DIV_ROUND_UP()` instead of truncating division. This matches what `framebuffer_check()` does at `drm_framebuffer.c:172-173`.

**Bug analysis is accurate.** The concrete scenario described — NV12 (vsub=2), height=1 — is a valid reproduction case:
- `framebuffer_check()` computes chroma height = `DIV_ROUND_UP(1, 2) = 1`, passes validation
- Old `drm_gem_fb_init_with_funcs()` computes chroma height = `1 / 2 = 0`
- `min_size = (0 - 1) * pitches[i] + ...` wraps `(unsigned int)(0 - 1)` = `0xFFFFFFFF`
- The multiply + add overflows `unsigned int`, potentially yielding a small `min_size`
- A too-small GEM object passes the size guard

**Issues to address:**

1. **Missing `Fixes:` tag.** This is a bug fix and should include a `Fixes:` tag referencing the commit that introduced this code (likely the original `drm_gem_fb_init_with_funcs()` or a refactor that introduced the open-coded division). This is important for backporting decisions.

2. **Missing `Cc: stable@vger.kernel.org`.** Given this is a potential out-of-bounds access triggered from userspace (via the `DRM_IOCTL_MODE_ADDFB2` path), this should be tagged for stable backports.

3. **Minor type mismatch.** `drm_format_info_plane_width()` and `drm_format_info_plane_height()` return `int`, but the local variables are `unsigned int`. This is a pre-existing pattern (the same implicit conversion happens at `drm_framebuffer.c:172-173`), so it's not introduced by this patch — but worth noting. The values will always be positive in practice since `framebuffer_check()` rejects `width==0` and `height==0` before this code runs.

4. **Commit message quality.** The commit message is well written and clearly explains the root cause, the concrete failure scenario, and the fix. No issues here.

**Summary:** The code change is correct, minimal, and addresses a real bug that could allow a malicious userspace to bypass the GEM object size check for sub-sampled formats. It should be resubmitted with `Fixes:` and `Cc: stable` tags added.

---
Generated by Claude Code Patch Reviewer