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