On Thu, Oct 28, 2021 at 05:04:19PM +0300, 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@xxxxxxxxxx>
> > ---
> > 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.
Yeah maybe even a compile-time check of the format table(s) to validate
them properly and scream ... Or at least a selftest.
-Daniel
>
> > +
> > 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
>
> --
> Ville Syrjälä
> Intel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
