A divide error bug in framebuffer_check

A divide error bug in framebuffer_check

[butt3rflyh4ck]

Hi, there is a divide error bug in framebuffer_check in
drivers/gpu/drm/drm_framebuffer.c in the latest kernel.
we can trigger it via drm_mode_addfb2 IOCTL.
The call trace is drm_mode_addfb2 -> drm_internal_framebuffer_create
->  framebuffer_check.
let us see code below:
```
static int framebuffer_check(struct drm_device *dev,
     const struct drm_mode_fb_cmd2 *r)
{
const struct drm_format_info *info;
int i;

/* check if the format is supported at all */
if (!__drm_format_info(r->pixel_format)) {
DRM_DEBUG_KMS("bad framebuffer format %p4cc\n",
      &r->pixel_format);
return -EINVAL;
}

if (r->width == 0) {
DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);
return -EINVAL;
}

if (r->height == 0) {
DRM_DEBUG_KMS("bad framebuffer height %u\n", r->height);
return -EINVAL;
}

/* now let the driver pick its own format info */
info = drm_get_format_info(dev, r);   //// [1] get info data

for (i = 0; i < info->num_planes; i++) {
unsigned int width = fb_plane_width(r->width, info, i);   ///  [2]
unsigned int height = fb_plane_height(r->height, info, i); /// [3]
unsigned int block_size = info->char_per_block[i];
u64 min_pitch = drm_format_info_min_pitch(info, i, width);
 ......
```
the drm_get_format_info gets info data and would call
__drm_format_info.see code below:
__drm_format_info
````
const struct drm_format_info *__drm_format_info(u32 format)
{
static const struct drm_format_info formats[] = {
 .....
{ .format = DRM_FORMAT_Q410, .depth = 0,
  .num_planes = 3, .char_per_block = { 2, 2, 2 },
  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
  .vsub = 0, .is_yuv = true },
{ .format = DRM_FORMAT_Q401, .depth = 0,
  .num_planes = 3, .char_per_block = { 2, 2, 2 },
  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
  .vsub = 0, .is_yuv = true },
......
}
unsigned int i;

for (i = 0; i < ARRAY_SIZE(formats); ++i) {
if (formats[i].format == format)
return &formats[i];
}

return NULL;
}
```
It would return a defined format.if format is that:
```
{ .format = DRM_FORMAT_Q410, .depth = 0,
  .num_planes = 3, .char_per_block = { 2, 2, 2 },
  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
  .vsub = 0, .is_yuv = true },
{ .format = DRM_FORMAT_Q401, .depth = 0,
  .num_planes = 3, .char_per_block = { 2, 2, 2 },
  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
  .vsub = 0, .is_yuv = true },
```
we can see format.hsub and format.vsub all are NULL.

[2] and [3], the fb_plane_width/height would process  info data, see
fb_plane_width code below:
```
static int fb_plane_width(int width,
  const struct drm_format_info *format, int plane)
{
if (plane == 0)
return width;

return DIV_ROUND_UP(width, format->hsub); //// [4]
}
```
[4] it would call DIV_ROUND_UP, but format->hsub is NULL.

##crash log
[  211.845762][ T4677] divide error: 0000 [#1] PREEMPT SMP
[  211.846194][ T4677] CPU: 1 PID: 4677 Comm: drm_internal_fr Not
tainted 6.0.0-rc5 #15
[  211.846732][ T4677] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996), BIOS 1.13.0-1ubuntu1 04/01/2014
[  211.847396][ T4677] RIP: 0010:drm_internal_framebuffer_create+0x151/0x6a0
[  211.847896][ T4677] Code: 00 0f b6 53 05 41 83 c5 01 41 39 d5 0f 8d
10 02 00 00 45 85 ed 45 8b 7c 24 04 0f 84 6b 01 00 00 0f b6 4b 12 41
8d 44 0f ff 99 <f7> f9 0f b1
[  211.849390][ T4677] RSP: 0018:ffffc9000aaafd18 EFLAGS: 00010202
[  211.849800][ T4677] RAX: 000000000000001e RBX: ffffffff83841768
RCX: 0000000000000000
[  211.850305][ T4677] RDX: 0000000000000000 RSI: 00000000ffffffff
RDI: 00000000ffffffff
[  211.850814][ T4677] RBP: 0000000000000000 R08: 0000000000000000
R09: 0000000000000002
[  211.851310][ T4677] R10: 0000000000000054 R11: 000000000007e2a0
R12: ffffc9000aaafe50
[  211.851810][ T4677] R13: 0000000000000001 R14: 0000000000001808
R15: 000000000000001f
[  211.852307][ T4677] FS:  0000000000dd7880(0000)
GS:ffff88807ec00000(0000) knlGS:0000000000000000
[  211.852885][ T4677] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  211.853277][ T4677] CR2: 0000000020000000 CR3: 0000000047fba000
CR4: 00000000000006e0
[  211.853783][ T4677] Call Trace:
[  211.854027][ T4677]  <TASK>
[  211.854244][ T4677]  ? find_held_lock+0x2b/0x80
[  211.854538][ T4677]  drm_mode_addfb2+0x2f/0xd0
[  211.854835][ T4677]  ? drm_mode_addfb_ioctl+0x10/0x10
[  211.855210][ T4677]  drm_ioctl_kernel+0xac/0x140
[  211.855501][ T4677]  drm_ioctl+0x21f/0x3e0
[  211.855753][ T4677]  ? drm_mode_addfb_ioctl+0x10/0x10
[  211.856058][ T4677]  __x64_sys_ioctl+0x7b/0xb0
[  211.856388][ T4677]  do_syscall_64+0x35/0xb0
[  211.856662][ T4677]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
[  211.857018][ T4677] RIP: 0033:0x44dcbd
[  211.857286][ T4677] Code: 28 c3 e8 36 29 00 00 66 0f 1f 44 00 00 f3
0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b
4c 24 08 0f 05 <48> 3d 01 f8
[  211.858603][ T4677] RSP: 002b:00007ffeee92f738 EFLAGS: 00000213
ORIG_RAX: 0000000000000010
[  211.859179][ T4677] RAX: ffffffffffffffda RBX: 0000000000400530
RCX: 000000000044dcbd
[  211.859705][ T4677] RDX: 0000000020000000 RSI: 00000000c06864b8
RDI: 0000000000000003
[  211.860211][ T4677] RBP: 00007ffeee92f750 R08: 0000000000000000
R09: 0000000000000000
[  211.860708][ T4677] R10: 000000000000ffff R11: 0000000000000213
R12: 0000000000403120
[  211.861206][ T4677] R13: 0000000000000000 R14: 00000000004c5018
R15: 0000000000000000


Regards,
 butt3rflyh4ck.


-- 
Active Defense Lab of Venustech

Re: A divide error bug in framebuffer_check

[Ville Syrjälä]

On Tue, Sep 13, 2022 at 01:49:40AM +0800, butt3rflyh4ck wrote:
> Hi, there is a divide error bug in framebuffer_check in
> drivers/gpu/drm/drm_framebuffer.c in the latest kernel.
> we can trigger it via drm_mode_addfb2 IOCTL.
> The call trace is drm_mode_addfb2 -> drm_internal_framebuffer_create
> ->  framebuffer_check.
> let us see code below:
> ```
<snip>
> { .format = DRM_FORMAT_Q410, .depth = 0,
>   .num_planes = 3, .char_per_block = { 2, 2, 2 },
>   .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
>   .vsub = 0, .is_yuv = true },
> { .format = DRM_FORMAT_Q401, .depth = 0,
>   .num_planes = 3, .char_per_block = { 2, 2, 2 },
>   .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
>   .vsub = 0, .is_yuv = true },
> ```
> we can see format.hsub and format.vsub all are NULL.

Yeah, those look borked.

Added in commit 94b292b27734 ("drm: drm_fourcc: add NV15, Q410, Q401 YUV formats")

Adding the relevant people to cc...

-- 
Ville Syrjälä
Intel

[PATCH] drm/fourcc: Fix vsub/hsub for Q410 and Q401

[Brian Starkey]

These formats are not subsampled, but that means hsub and vsub should be
1, not 0.

Fixes: 94b292b27734 ("drm: drm_fourcc: add NV15, Q410, Q401 YUV formats")
Reported-by: George Kennedy <george.kennedy@oracle.com>
Reported-by: butt3rflyh4ck <butterflyhuangxx@gmail.com>
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 drivers/gpu/drm/drm_fourcc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Sorry, I lost track of this - I thought it got fixed after the previous
thread[1].

-Brian

[1] https://lore.kernel.org/all/26fdb955-10c8-a5d6-07b6-85a4374e7754@oracle.com/

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 07741b678798..6768b7d18b6f 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -263,12 +263,12 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		  .vsub = 2, .is_yuv = true },
 		{ .format = DRM_FORMAT_Q410,		.depth = 0,
 		  .num_planes = 3, .char_per_block = { 2, 2, 2 },
-		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
-		  .vsub = 0, .is_yuv = true },
+		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 1,
+		  .vsub = 1, .is_yuv = true },
 		{ .format = DRM_FORMAT_Q401,		.depth = 0,
 		  .num_planes = 3, .char_per_block = { 2, 2, 2 },
-		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
-		  .vsub = 0, .is_yuv = true },
+		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 1,
+		  .vsub = 1, .is_yuv = true },
 		{ .format = DRM_FORMAT_P030,            .depth = 0,  .num_planes = 2,
 		  .char_per_block = { 4, 8, 0 }, .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 },
 		  .hsub = 2, .vsub = 2, .is_yuv = true},
-- 
2.25.1

Re: [PATCH] drm/fourcc: Fix vsub/hsub for Q410 and Q401

[Liviu Dudau]

On Tue, Sep 13, 2022 at 03:43:06PM +0100, Brian Starkey wrote:
> These formats are not subsampled, but that means hsub and vsub should be
> 1, not 0.
> 
> Fixes: 94b292b27734 ("drm: drm_fourcc: add NV15, Q410, Q401 YUV formats")
> Reported-by: George Kennedy <george.kennedy@oracle.com>
> Reported-by: butt3rflyh4ck <butterflyhuangxx@gmail.com>
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Should this be backported into stable releases? How far back to we go?

Best regards,
Liviu


> ---
>  drivers/gpu/drm/drm_fourcc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Sorry, I lost track of this - I thought it got fixed after the previous
> thread[1].
> 
> -Brian
> 
> [1] https://lore.kernel.org/all/26fdb955-10c8-a5d6-07b6-85a4374e7754@oracle.com/
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 07741b678798..6768b7d18b6f 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -263,12 +263,12 @@ const struct drm_format_info *__drm_format_info(u32 format)
>  		  .vsub = 2, .is_yuv = true },
>  		{ .format = DRM_FORMAT_Q410,		.depth = 0,
>  		  .num_planes = 3, .char_per_block = { 2, 2, 2 },
> -		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
> -		  .vsub = 0, .is_yuv = true },
> +		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 1,
> +		  .vsub = 1, .is_yuv = true },
>  		{ .format = DRM_FORMAT_Q401,		.depth = 0,
>  		  .num_planes = 3, .char_per_block = { 2, 2, 2 },
> -		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
> -		  .vsub = 0, .is_yuv = true },
> +		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 1,
> +		  .vsub = 1, .is_yuv = true },
>  		{ .format = DRM_FORMAT_P030,            .depth = 0,  .num_planes = 2,
>  		  .char_per_block = { 4, 8, 0 }, .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 },
>  		  .hsub = 2, .vsub = 2, .is_yuv = true},
> -- 
> 2.25.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH] drm/fourcc: Fix vsub/hsub for Q410 and Q401

[Brian Starkey]

On Tue, Sep 13, 2022 at 04:36:57PM +0100, Liviu Dudau wrote:
> On Tue, Sep 13, 2022 at 03:43:06PM +0100, Brian Starkey wrote:
> > These formats are not subsampled, but that means hsub and vsub should be
> > 1, not 0.
> > 
> > Fixes: 94b292b27734 ("drm: drm_fourcc: add NV15, Q410, Q401 YUV formats")
> > Reported-by: George Kennedy <george.kennedy@oracle.com>
> > Reported-by: butt3rflyh4ck <butterflyhuangxx@gmail.com>
> > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> 
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> 
> Should this be backported into stable releases? How far back to we go?

Probably, git says 94b292b27734 is in since 5.10.

Could someone merge this so it doesn't get lost again?

Thanks,
-Brian

> 
> Best regards,
> Liviu
> 
> 
> > ---
> >  drivers/gpu/drm/drm_fourcc.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > Sorry, I lost track of this - I thought it got fixed after the previous
> > thread[1].
> > 
> > -Brian
> > 
> > [1] https://lore.kernel.org/all/26fdb955-10c8-a5d6-07b6-85a4374e7754@oracle.com/
> > 
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index 07741b678798..6768b7d18b6f 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -263,12 +263,12 @@ const struct drm_format_info *__drm_format_info(u32 format)
> >  		  .vsub = 2, .is_yuv = true },
> >  		{ .format = DRM_FORMAT_Q410,		.depth = 0,
> >  		  .num_planes = 3, .char_per_block = { 2, 2, 2 },
> > -		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
> > -		  .vsub = 0, .is_yuv = true },
> > +		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 1,
> > +		  .vsub = 1, .is_yuv = true },
> >  		{ .format = DRM_FORMAT_Q401,		.depth = 0,
> >  		  .num_planes = 3, .char_per_block = { 2, 2, 2 },
> > -		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
> > -		  .vsub = 0, .is_yuv = true },
> > +		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 1,
> > +		  .vsub = 1, .is_yuv = true },
> >  		{ .format = DRM_FORMAT_P030,            .depth = 0,  .num_planes = 2,
> >  		  .char_per_block = { 4, 8, 0 }, .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 },
> >  		  .hsub = 2, .vsub = 2, .is_yuv = true},
> > -- 
> > 2.25.1
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

Re: [PATCH] drm/fourcc: Fix vsub/hsub for Q410 and Q401

[Liviu Dudau]

On Mon, Sep 26, 2022 at 04:21:19PM +0100, Brian Starkey wrote:
> On Tue, Sep 13, 2022 at 04:36:57PM +0100, Liviu Dudau wrote:
> > On Tue, Sep 13, 2022 at 03:43:06PM +0100, Brian Starkey wrote:
> > > These formats are not subsampled, but that means hsub and vsub should be
> > > 1, not 0.
> > > 
> > > Fixes: 94b292b27734 ("drm: drm_fourcc: add NV15, Q410, Q401 YUV formats")
> > > Reported-by: George Kennedy <george.kennedy@oracle.com>
> > > Reported-by: butt3rflyh4ck <butterflyhuangxx@gmail.com>
> > > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > 
> > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > 
> > Should this be backported into stable releases? How far back to we go?
> 
> Probably, git says 94b292b27734 is in since 5.10.
> 
> Could someone merge this so it doesn't get lost again?

I'll merge this into drm-misc-next-fixes this week and notify stable about it.

Best regards,
Liviu

> 
> Thanks,
> -Brian
> 
> > 
> > Best regards,
> > Liviu
> > 
> > 
> > > ---
> > >  drivers/gpu/drm/drm_fourcc.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > Sorry, I lost track of this - I thought it got fixed after the previous
> > > thread[1].
> > > 
> > > -Brian
> > > 
> > > [1] https://lore.kernel.org/all/26fdb955-10c8-a5d6-07b6-85a4374e7754@oracle.com/
> > > 
> > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > > index 07741b678798..6768b7d18b6f 100644
> > > --- a/drivers/gpu/drm/drm_fourcc.c
> > > +++ b/drivers/gpu/drm/drm_fourcc.c
> > > @@ -263,12 +263,12 @@ const struct drm_format_info *__drm_format_info(u32 format)
> > >  		  .vsub = 2, .is_yuv = true },
> > >  		{ .format = DRM_FORMAT_Q410,		.depth = 0,
> > >  		  .num_planes = 3, .char_per_block = { 2, 2, 2 },
> > > -		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
> > > -		  .vsub = 0, .is_yuv = true },
> > > +		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 1,
> > > +		  .vsub = 1, .is_yuv = true },
> > >  		{ .format = DRM_FORMAT_Q401,		.depth = 0,
> > >  		  .num_planes = 3, .char_per_block = { 2, 2, 2 },
> > > -		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0,
> > > -		  .vsub = 0, .is_yuv = true },
> > > +		  .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 1,
> > > +		  .vsub = 1, .is_yuv = true },
> > >  		{ .format = DRM_FORMAT_P030,            .depth = 0,  .num_planes = 2,
> > >  		  .char_per_block = { 4, 8, 0 }, .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 },
> > >  		  .hsub = 2, .vsub = 2, .is_yuv = true},
> > > -- 
> > > 2.25.1
> > > 
> > 
> > -- 
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯