From: daniel@ffwll.ch
Cc: Linux Fbdev development list <linux-fbdev@vger.kernel.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Jiri Slaby <jslaby@suse.com>,
syzbot <syzbot+c37a14770d51a085a520@syzkaller.appspotmail.com>
Subject: Re: [PATCH v2] fbmem: pull fbcon_update_vcs() out of fb_set_var()
Date: Tue, 4 Aug 2020 07:38:14 +0200 [thread overview]
Message-ID: <20200804053814.GG6419@phenom.ffwll.local> (raw)
In-Reply-To: <075b7e37-3278-cd7d-31ab-c5073cfa8e92@i-love.sakura.ne.jp>
On Thu, Jul 30, 2020 at 07:47:14PM +0900, Tetsuo Handa wrote:
> syzbot is reporting OOB read bug in vc_do_resize() [1] caused by memcpy()
> based on outdated old_{rows,row_size} values, for resize_screen() can
> recurse into vc_do_resize() which changes vc->vc_{cols,rows} that outdates
> old_{rows,row_size} values which were saved before calling resize_screen().
>
> Daniel Vetter explained that resize_screen() should not recurse into
> fbcon_update_vcs() path due to FBINFO_MISC_USEREVENT being still set
> when calling resize_screen().
>
> Instead of masking FBINFO_MISC_USEREVENT before calling fbcon_update_vcs(),
> we can remove FBINFO_MISC_USEREVENT by calling fbcon_update_vcs() only if
> fb_set_var() returned 0. This change assumes that it is harmless to call
> fbcon_update_vcs() when fb_set_var() returned 0 without reaching
> fb_notifier_call_chain().
>
> [1] https://syzkaller.appspot.com/bug?id=c70c88cfd16dcf6e1d3c7f0ab8648b3144b5b25e
>
> Reported-and-tested-by: syzbot <syzbot+c37a14770d51a085a520@syzkaller.appspotmail.com>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Reported-by: kernel test robot <lkp@intel.com> for missing #include
Thanks a lot for your patch, queued up to hopefully still make it for
5.9-rc1.
Cheers, Daniel
> ---
> drivers/video/fbdev/core/fbmem.c | 8 ++------
> drivers/video/fbdev/core/fbsysfs.c | 4 ++--
> drivers/video/fbdev/ps3fb.c | 5 +++--
> include/linux/fb.h | 2 --
> 4 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 30e73ec..da7c88f 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -957,7 +957,6 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
> int
> fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
> {
> - int flags = info->flags;
> int ret = 0;
> u32 activate;
> struct fb_var_screeninfo old_var;
> @@ -1052,9 +1051,6 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
> event.data = &mode;
> fb_notifier_call_chain(FB_EVENT_MODE_CHANGE, &event);
>
> - if (flags & FBINFO_MISC_USEREVENT)
> - fbcon_update_vcs(info, activate & FB_ACTIVATE_ALL);
> -
> return 0;
> }
> EXPORT_SYMBOL(fb_set_var);
> @@ -1105,9 +1101,9 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> return -EFAULT;
> console_lock();
> lock_fb_info(info);
> - info->flags |= FBINFO_MISC_USEREVENT;
> ret = fb_set_var(info, &var);
> - info->flags &= ~FBINFO_MISC_USEREVENT;
> + if (!ret)
> + fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
> unlock_fb_info(info);
> console_unlock();
> if (!ret && copy_to_user(argp, &var, sizeof(var)))
> diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
> index d54c88f..65dae05 100644
> --- a/drivers/video/fbdev/core/fbsysfs.c
> +++ b/drivers/video/fbdev/core/fbsysfs.c
> @@ -91,9 +91,9 @@ static int activate(struct fb_info *fb_info, struct fb_var_screeninfo *var)
>
> var->activate |= FB_ACTIVATE_FORCE;
> console_lock();
> - fb_info->flags |= FBINFO_MISC_USEREVENT;
> err = fb_set_var(fb_info, var);
> - fb_info->flags &= ~FBINFO_MISC_USEREVENT;
> + if (!err)
> + fbcon_update_vcs(fb_info, var->activate & FB_ACTIVATE_ALL);
> console_unlock();
> if (err)
> return err;
> diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c
> index 9df78fb..203c254 100644
> --- a/drivers/video/fbdev/ps3fb.c
> +++ b/drivers/video/fbdev/ps3fb.c
> @@ -29,6 +29,7 @@
> #include <linux/freezer.h>
> #include <linux/uaccess.h>
> #include <linux/fb.h>
> +#include <linux/fbcon.h>
> #include <linux/init.h>
>
> #include <asm/cell-regs.h>
> @@ -824,12 +825,12 @@ static int ps3fb_ioctl(struct fb_info *info, unsigned int cmd,
> var = info->var;
> fb_videomode_to_var(&var, vmode);
> console_lock();
> - info->flags |= FBINFO_MISC_USEREVENT;
> /* Force, in case only special bits changed */
> var.activate |= FB_ACTIVATE_FORCE;
> par->new_mode_id = val;
> retval = fb_set_var(info, &var);
> - info->flags &= ~FBINFO_MISC_USEREVENT;
> + if (!retval)
> + fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
> console_unlock();
> }
> break;
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 3b4b2f0..b11eb02 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -400,8 +400,6 @@ struct fb_tile_ops {
> #define FBINFO_HWACCEL_YPAN 0x2000 /* optional */
> #define FBINFO_HWACCEL_YWRAP 0x4000 /* optional */
>
> -#define FBINFO_MISC_USEREVENT 0x10000 /* event request
> - from userspace */
> #define FBINFO_MISC_TILEBLITTING 0x20000 /* use tile blitting */
>
> /* A driver may set this flag to indicate that it does want a set_par to be
> --
> 1.8.3.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-08-04 5:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-29 5:30 [PATCH] vt: Handle recursion in vc_do_resize() Tetsuo Handa
2020-07-29 8:18 ` Daniel Vetter
2020-07-29 22:46 ` [PATCH] fbmem: pull fbcon_update_vcs() out of fb_set_var() Tetsuo Handa
2020-07-30 10:47 ` [PATCH v2] " Tetsuo Handa
2020-08-04 5:38 ` daniel [this message]
2020-07-30 11:16 ` [PATCH] " Daniel Vetter
2020-07-30 11:27 ` Tetsuo Handa
2020-07-30 11:35 ` Daniel Vetter
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=20200804053814.GG6419@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=b.zolnierkie@samsung.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.com \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=syzbot+c37a14770d51a085a520@syzkaller.appspotmail.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).