Messages in this thread | | | Date | Fri, 29 Oct 2021 09:15:28 -0400 | Subject | Re: [PATCH] drm: check drm_format_info hsub and vsub to avoid divide by zero | From | George Kennedy <> |
| |
On 10/28/2021 10:04 AM, Ville Syrjälä wrote: > On Thu, Oct 28, 2021 at 08:57:17AM -0500, George Kennedy wrote: >> Do a sanity check on struct drm_format_info hsub and vsub values to >> avoid divide by zero. >> >> Syzkaller reported a divide error in framebuffer_check() when the >> DRM_FORMAT_Q410 or DRM_FORMAT_Q401 pixel_format is passed in via >> the DRM_IOCTL_MODE_ADDFB2 ioctl. The drm_format_info struct for >> the DRM_FORMAT_Q410 pixel_pattern has ".hsub = 0" and ".vsub = 0". >> fb_plane_width() uses hsub as a divisor and fb_plane_height() uses >> vsub as a divisor. These divisors need to be sanity checked for >> zero before use. >> >> divide error: 0000 [#1] SMP KASAN NOPTI >> CPU: 0 PID: 14995 Comm: syz-executor709 Not tainted 5.15.0-rc6-syzk #1 >> Hardware name: Red Hat KVM, BIOS 1.13.0-2 >> RIP: 0010:framebuffer_check drivers/gpu/drm/drm_framebuffer.c:199 [inline] >> RIP: 0010:drm_internal_framebuffer_create+0x604/0xf90 >> drivers/gpu/drm/drm_framebuffer.c:317 >> >> Call Trace: >> drm_mode_addfb2+0xdc/0x320 drivers/gpu/drm/drm_framebuffer.c:355 >> drm_mode_addfb2_ioctl+0x2a/0x40 drivers/gpu/drm/drm_framebuffer.c:391 >> drm_ioctl_kernel+0x23a/0x2e0 drivers/gpu/drm/drm_ioctl.c:795 >> drm_ioctl+0x589/0xac0 drivers/gpu/drm/drm_ioctl.c:898 >> vfs_ioctl fs/ioctl.c:51 [inline] >> __do_sys_ioctl fs/ioctl.c:874 [inline] >> __se_sys_ioctl fs/ioctl.c:860 [inline] >> __x64_sys_ioctl+0x19d/0x220 fs/ioctl.c:860 >> do_syscall_x64 arch/x86/entry/common.c:50 [inline] >> do_syscall_64+0x3a/0x80 arch/x86/entry/common.c:80 >> entry_SYSCALL_64_after_hwframe+0x44/0xae >> >> Signed-off-by: George Kennedy <george.kennedy@oracle.com> >> --- >> drivers/gpu/drm/drm_framebuffer.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c >> index 07f5abc..a146e4b 100644 >> --- a/drivers/gpu/drm/drm_framebuffer.c >> +++ b/drivers/gpu/drm/drm_framebuffer.c >> @@ -195,6 +195,16 @@ static int framebuffer_check(struct drm_device *dev, >> /* now let the driver pick its own format info */ >> info = drm_get_format_info(dev, r); >> >> + if (info->hsub == 0) { >> + DRM_DEBUG_KMS("bad horizontal chroma subsampling factor %u\n", info->hsub); >> + return -EINVAL; >> + } >> + >> + if (info->vsub == 0) { >> + DRM_DEBUG_KMS("bad vertical chroma subsampling factor %u\n", info->vsub); >> + return -EINVAL; >> + } > Looks like duct tape to me. I think we need to either fix those formats > to have valid format info, or just revert the whole patch that added such > broken things.
Adding the authors and reviewer of the patch in question to CC.
94b292b277343190175d39172c903c0c5fb814f1 drm: drm_fourcc: add NV15, Q410, Q401 YUV formats
Asking if you have any input on how to deal with hsub and vsub = zero?
The sanity checks of hsub and vsub in the proposed patch are similar to other error checking done in framebuffer_check() preceding the proposed patch.
Thank you, George > >> + >> for (i = 0; i < info->num_planes; i++) { >> unsigned int width = fb_plane_width(r->width, info, i); >> unsigned int height = fb_plane_height(r->height, info, i); >> -- >> 1.8.3.1
| |