From: George Kennedy <george.kennedy@oracle.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: gregkh@linuxfoundation.org, maarten.lankhorst@linux.intel.com,
mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie,
daniel@ffwll.ch, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, ben.davis@arm.com,
brian.starkey@arm.com, Liviu.Dudau@arm.com,
Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH] drm: check drm_format_info hsub and vsub to avoid divide by zero
Date: Fri, 29 Oct 2021 09:15:28 -0400 [thread overview]
Message-ID: <26fdb955-10c8-a5d6-07b6-85a4374e7754@oracle.com> (raw)
In-Reply-To: <YXqt46TPL9tUZCL1@intel.com>
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
next prev parent reply other threads:[~2021-10-29 13:16 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-28 13:57 [PATCH] drm: check drm_format_info hsub and vsub to avoid divide by zero George Kennedy
2021-10-28 14:04 ` Ville Syrjälä
2021-10-29 13:15 ` George Kennedy [this message]
2021-10-29 14:14 ` Brian Starkey
2021-11-18 20:00 ` George Kennedy
2021-11-19 9:42 ` Daniel Vetter
2021-11-19 9:40 ` Daniel Vetter
2021-11-19 10:03 ` Ville Syrjälä
2021-11-19 10:30 ` Daniel Vetter
2021-11-19 14:25 ` Jani Nikula
2021-11-22 15:29 ` George Kennedy
2021-11-25 15:27 ` Daniel Vetter
2021-12-02 14:17 ` George Kennedy
2021-12-07 17:57 ` Daniel Vetter
2021-10-28 14:09 ` Simon Ser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=26fdb955-10c8-a5d6-07b6-85a4374e7754@oracle.com \
--to=george.kennedy@oracle.com \
--cc=Liviu.Dudau@arm.com \
--cc=airlied@linux.ie \
--cc=ben.davis@arm.com \
--cc=brian.starkey@arm.com \
--cc=dan.carpenter@oracle.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=tzimmermann@suse.de \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).