From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> To: Linus Torvalds <torvalds@linux-foundation.org>, "Maciej W. Rozycki" <macro@orcam.me.uk> Cc: dri-devel <dri-devel@lists.freedesktop.org>, Linux Fbdev development list <linux-fbdev@vger.kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Daniel Vetter <daniel@ffwll.ch>, syzbot <syzbot+1f29e126cf461c4de3b3@syzkaller.appspotmail.com>, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, Colin King <colin.king@canonical.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Jani Nikula <jani.nikula@intel.com>, Jiri Slaby <jirislaby@kernel.org>, syzkaller-bugs <syzkaller-bugs@googlegroups.com>, "Antonino A. Daplas" <adaplas@gmail.com> Subject: [PATCH v2] tty: vt: always invoke vc->vc_sw->con_resize callback Date: Sat, 15 May 2021 16:43:49 +0900 [thread overview] Message-ID: <97f1d292-c3a8-f4d6-0651-b4f5571ecb72@i-love.sakura.ne.jp> (raw) In-Reply-To: <CAHk-=wjkVAjfWrmmJnJe1_MriK9gezWCew_MU=MbQNzHbGopsQ@mail.gmail.com> On 2021/05/15 1:19, Tetsuo Handa wrote: > Even if it turns out to be safe to always call this > callback, we will need to involve another callback via "struct fb_ops" for > checking the upper limits from fbcon_resize(). As a result, we will need > to modify > > drivers/tty/vt/vt.c > drivers/video/fbdev/core/fbcon.c > drivers/video/fbdev/vga16fb.c > include/linux/fb.h > > files only for checking rows/columns values passed to ioctl(VT_RESIZE) > request. I was by error assuming that fbcon_resize() cannot reject bogus rows/columns and thus we need to add another callback via "struct fb_ops" for that purpose. But fbcon_resize() does reject bogus rows/columns; it was simply because resize_screen() did not call fbcon_resize() if vc->vc_mode == KD_GRAPHICS. Thus, removing vc->vc_mode check alone is sufficient. On 2021/05/15 6:10, Linus Torvalds wrote: > So I think just removing the "vc->vc_mode != KD_GRAPHICS" test from > resize_screen() might be the way to go. That way, the low-level data > structures actually are in sync with the resize, and the "out of > bounds" bug should never happen. > > Would you mind testing that? OK. Your suggested changes passed the test by me and by syzbot. From e5e326c90c5b919c6aba30072d665a00b18715a5 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Sat, 15 May 2021 03:00:37 +0000 Subject: [PATCH v2] tty: vt: always invoke vc->vc_sw->con_resize callback syzbot is reporting OOB write at vga16fb_imageblit() [1], for resize_screen() from ioctl(VT_RESIZE) returns 0 without checking whether requested rows/columns fit the amount of memory reserved for the graphical screen if current mode is KD_GRAPHICS. ---------- #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> #include <sys/ioctl.h> #include <linux/kd.h> #include <linux/vt.h> int main(int argc, char *argv[]) { const int fd = open("/dev/char/4:1", O_RDWR); struct vt_sizes vt = { 0x4100, 2 }; ioctl(fd, KDSETMODE, KD_GRAPHICS); ioctl(fd, VT_RESIZE, &vt); ioctl(fd, KDSETMODE, KD_TEXT); return 0; } ---------- Allow framebuffer drivers to return -EINVAL, by moving vc->vc_mode != KD_GRAPHICS check from resize_screen() to fbcon_resize(). [1] https://syzkaller.appspot.com/bug?extid=1f29e126cf461c4de3b3 Reported-by: syzbot <syzbot+1f29e126cf461c4de3b3@syzkaller.appspotmail.com> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Tested-by: syzbot <syzbot+1f29e126cf461c4de3b3@syzkaller.appspotmail.com> --- drivers/tty/vt/vt.c | 2 +- drivers/video/fbdev/core/fbcon.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 01645e87b3d5..fa1548d4f94b 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -1171,7 +1171,7 @@ static inline int resize_screen(struct vc_data *vc, int width, int height, /* Resizes the resolution of the display adapater */ int err = 0; - if (vc->vc_mode != KD_GRAPHICS && vc->vc_sw->con_resize) + if (vc->vc_sw->con_resize) err = vc->vc_sw->con_resize(vc, width, height, user); return err; diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 3406067985b1..22bb3892f6bd 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -2019,7 +2019,7 @@ static int fbcon_resize(struct vc_data *vc, unsigned int width, return -EINVAL; pr_debug("resize now %ix%i\n", var.xres, var.yres); - if (con_is_visible(vc)) { + if (con_is_visible(vc) && vc->vc_mode == KD_TEXT) { var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE; fb_set_var(info, &var); -- 2.25.1
WARNING: multiple messages have this Message-ID (diff)
From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> To: Linus Torvalds <torvalds@linux-foundation.org>, "Maciej W. Rozycki" <macro@orcam.me.uk> Cc: Linux Fbdev development list <linux-fbdev@vger.kernel.org>, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, Jani Nikula <jani.nikula@intel.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, syzkaller-bugs <syzkaller-bugs@googlegroups.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, dri-devel <dri-devel@lists.freedesktop.org>, Colin King <colin.king@canonical.com>, Jiri Slaby <jirislaby@kernel.org>, syzbot <syzbot+1f29e126cf461c4de3b3@syzkaller.appspotmail.com> Subject: [PATCH v2] tty: vt: always invoke vc->vc_sw->con_resize callback Date: Sat, 15 May 2021 16:43:49 +0900 [thread overview] Message-ID: <97f1d292-c3a8-f4d6-0651-b4f5571ecb72@i-love.sakura.ne.jp> (raw) In-Reply-To: <CAHk-=wjkVAjfWrmmJnJe1_MriK9gezWCew_MU=MbQNzHbGopsQ@mail.gmail.com> On 2021/05/15 1:19, Tetsuo Handa wrote: > Even if it turns out to be safe to always call this > callback, we will need to involve another callback via "struct fb_ops" for > checking the upper limits from fbcon_resize(). As a result, we will need > to modify > > drivers/tty/vt/vt.c > drivers/video/fbdev/core/fbcon.c > drivers/video/fbdev/vga16fb.c > include/linux/fb.h > > files only for checking rows/columns values passed to ioctl(VT_RESIZE) > request. I was by error assuming that fbcon_resize() cannot reject bogus rows/columns and thus we need to add another callback via "struct fb_ops" for that purpose. But fbcon_resize() does reject bogus rows/columns; it was simply because resize_screen() did not call fbcon_resize() if vc->vc_mode == KD_GRAPHICS. Thus, removing vc->vc_mode check alone is sufficient. On 2021/05/15 6:10, Linus Torvalds wrote: > So I think just removing the "vc->vc_mode != KD_GRAPHICS" test from > resize_screen() might be the way to go. That way, the low-level data > structures actually are in sync with the resize, and the "out of > bounds" bug should never happen. > > Would you mind testing that? OK. Your suggested changes passed the test by me and by syzbot. From e5e326c90c5b919c6aba30072d665a00b18715a5 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Sat, 15 May 2021 03:00:37 +0000 Subject: [PATCH v2] tty: vt: always invoke vc->vc_sw->con_resize callback syzbot is reporting OOB write at vga16fb_imageblit() [1], for resize_screen() from ioctl(VT_RESIZE) returns 0 without checking whether requested rows/columns fit the amount of memory reserved for the graphical screen if current mode is KD_GRAPHICS. ---------- #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> #include <sys/ioctl.h> #include <linux/kd.h> #include <linux/vt.h> int main(int argc, char *argv[]) { const int fd = open("/dev/char/4:1", O_RDWR); struct vt_sizes vt = { 0x4100, 2 }; ioctl(fd, KDSETMODE, KD_GRAPHICS); ioctl(fd, VT_RESIZE, &vt); ioctl(fd, KDSETMODE, KD_TEXT); return 0; } ---------- Allow framebuffer drivers to return -EINVAL, by moving vc->vc_mode != KD_GRAPHICS check from resize_screen() to fbcon_resize(). [1] https://syzkaller.appspot.com/bug?extid=1f29e126cf461c4de3b3 Reported-by: syzbot <syzbot+1f29e126cf461c4de3b3@syzkaller.appspotmail.com> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Tested-by: syzbot <syzbot+1f29e126cf461c4de3b3@syzkaller.appspotmail.com> --- drivers/tty/vt/vt.c | 2 +- drivers/video/fbdev/core/fbcon.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 01645e87b3d5..fa1548d4f94b 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -1171,7 +1171,7 @@ static inline int resize_screen(struct vc_data *vc, int width, int height, /* Resizes the resolution of the display adapater */ int err = 0; - if (vc->vc_mode != KD_GRAPHICS && vc->vc_sw->con_resize) + if (vc->vc_sw->con_resize) err = vc->vc_sw->con_resize(vc, width, height, user); return err; diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 3406067985b1..22bb3892f6bd 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -2019,7 +2019,7 @@ static int fbcon_resize(struct vc_data *vc, unsigned int width, return -EINVAL; pr_debug("resize now %ix%i\n", var.xres, var.yres); - if (con_is_visible(vc)) { + if (con_is_visible(vc) && vc->vc_mode == KD_TEXT) { var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE; fb_set_var(info, &var); -- 2.25.1
next prev parent reply other threads:[~2021-05-15 7:44 UTC|newest] Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-08 7:07 BUG: unable to handle kernel paging request in vga16fb_imageblit (2) syzbot 2020-05-08 7:07 ` syzbot 2020-05-08 7:07 ` syzbot 2021-05-01 20:31 ` [syzbot] " syzbot 2021-05-01 20:31 ` syzbot 2021-05-02 1:53 ` syzbot 2021-05-02 1:53 ` syzbot 2021-05-03 13:41 ` Tetsuo Handa 2021-05-03 13:41 ` Tetsuo Handa 2021-05-07 11:09 ` Tetsuo Handa 2021-05-14 16:19 ` [PATCH] video: fbdev: vga16fb: fix OOB write in vga16fb_imageblit() Tetsuo Handa 2021-05-14 16:19 ` Tetsuo Handa 2021-05-14 17:29 ` Linus Torvalds 2021-05-14 17:29 ` Linus Torvalds 2021-05-14 17:37 ` Linus Torvalds 2021-05-14 17:37 ` Linus Torvalds 2021-05-14 18:23 ` Linus Torvalds 2021-05-14 18:23 ` Linus Torvalds 2021-05-14 20:25 ` Maciej W. Rozycki 2021-05-14 20:25 ` Maciej W. Rozycki 2021-05-14 20:32 ` Linus Torvalds 2021-05-14 20:32 ` Linus Torvalds 2021-05-14 21:10 ` Linus Torvalds 2021-05-14 21:10 ` Linus Torvalds 2021-05-15 7:43 ` Tetsuo Handa [this message] 2021-05-15 7:43 ` [PATCH v2] tty: vt: always invoke vc->vc_sw->con_resize callback Tetsuo Handa 2021-05-15 16:21 ` Maciej W. Rozycki 2021-05-15 16:21 ` Maciej W. Rozycki 2021-05-15 16:32 ` Maciej W. Rozycki 2021-05-15 16:32 ` Maciej W. Rozycki 2021-05-15 16:41 ` Linus Torvalds 2021-05-15 16:41 ` Linus Torvalds 2021-05-17 13:13 ` Daniel Vetter 2021-05-17 13:13 ` Daniel Vetter 2021-05-15 16:11 ` [PATCH] video: fbdev: vga16fb: fix OOB write in vga16fb_imageblit() Maciej W. Rozycki 2021-05-15 16:11 ` Maciej W. Rozycki 2021-05-17 13:07 ` Daniel Vetter 2021-05-17 13:07 ` Daniel Vetter 2021-05-17 13:10 ` Daniel Vetter 2021-05-17 13:10 ` Daniel Vetter 2021-05-15 0:45 ` Tetsuo Handa 2021-05-15 0:45 ` Tetsuo Handa -- strict thread matches above, loose matches on Subject: below -- 2020-10-06 8:18 BUG: unable to handle kernel paging request in cfb_imageblit syzbot 2020-10-06 8:18 ` syzbot 2020-10-06 8:18 ` syzbot 2020-12-18 15:26 ` syzbot 2020-12-18 15:26 ` syzbot 2020-12-18 15:27 ` Dmitry Vyukov 2020-12-18 15:27 ` Dmitry Vyukov 2020-05-12 6:55 BUG: unable to handle kernel paging request in bitfill_aligned syzbot 2020-05-12 6:55 ` syzbot 2020-05-12 6:55 ` syzbot 2019-12-27 7:13 BUG: unable to handle kernel paging request in vga16fb_imageblit syzbot 2019-12-27 7:13 ` syzbot 2019-12-27 7:13 ` syzbot 2019-12-10 16:38 BUG: unable to handle kernel paging request in sys_imageblit syzbot 2019-12-10 16:38 ` syzbot 2019-12-10 16:38 ` syzbot 2020-06-19 4:56 ` syzbot 2020-06-19 4:56 ` syzbot 2020-06-19 4:56 ` syzbot 2013-02-19 17:33 BUG: unable to handle kernel paging request at ffffc90000669000, IP: [<ffffffff8139d84a>] bitfill_unaligned+0x10a/0x1a0 Tommi Rantala 2013-02-19 17:33 ` BUG: unable to handle kernel paging request at ffffc90000669000, IP: [<ffffffff8139d84a>] bitfill_un Tommi Rantala
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=97f1d292-c3a8-f4d6-0651-b4f5571ecb72@i-love.sakura.ne.jp \ --to=penguin-kernel@i-love.sakura.ne.jp \ --cc=adaplas@gmail.com \ --cc=b.zolnierkie@samsung.com \ --cc=colin.king@canonical.com \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=gregkh@linuxfoundation.org \ --cc=jani.nikula@intel.com \ --cc=jirislaby@kernel.org \ --cc=linux-fbdev@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=macro@orcam.me.uk \ --cc=syzbot+1f29e126cf461c4de3b3@syzkaller.appspotmail.com \ --cc=syzkaller-bugs@googlegroups.com \ --cc=torvalds@linux-foundation.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.