tty: vt: always invoke vc->vc_sw->con_resize callback

Started by Sasha Levin on May 18, 2021 · 6 messages from 3 authors · Last reply May 24, 2021

Subsystems (5)

Show 2 more

Maintainers (6)

Show 2 more
STALE1997d REVIEWED · 1 review, 0 by maintainers

Status: Reviewed.

1 review trailer.

open the first message on its own page

Message details and related content
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

[ Upstream commit ffb324e6f874121f7dce5bdae5e05d02baae7269 ]

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().

Link: https://syzkaller.appspot.com/bug?extid=1f29e126cf461c4de3b3 [1]
Reported-by: syzbot <redacted>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Tested-by: syzbot <redacted>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

4 more lines

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 0cc360da5426..53cbf2c3f033 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.30.2
Message details and related content
On Mon, May 17, 2021 at 6:09 PM Sasha Levin [off-list ref] wrote:
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

[ Upstream commit ffb324e6f874121f7dce5bdae5e05d02baae7269 ]
So I think the commit is fine, and yes, it should be applied to
stable, but it's one of those "there were three different patches in
as many days to fix the problem, and this is the right one, but maybe
stable should hold off for a while to see that there aren't any
problem reports".

I don't think there will be any problems from this, but while the
patch is tiny, it's conceptually quite a big change to something that
people haven't really touched for a long time.

So use your own judgement, but it might be a good idea to wait a week
before backporting this to see if anything screams.

          Linus
Message details and related content
On Mon, May 17, 2021 at 06:35:24PM -0700, Linus Torvalds wrote:
On Mon, May 17, 2021 at 6:09 PM Sasha Levin [off-list ref] wrote:
quoted
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

[ Upstream commit ffb324e6f874121f7dce5bdae5e05d02baae7269 ]
So I think the commit is fine, and yes, it should be applied to
stable, but it's one of those "there were three different patches in
as many days to fix the problem, and this is the right one, but maybe
stable should hold off for a while to see that there aren't any
problem reports".

I don't think there will be any problems from this, but while the
patch is tiny, it's conceptually quite a big change to something that
people haven't really touched for a long time.

So use your own judgement, but it might be a good idea to wait a week
before backporting this to see if anything screams.
I was going to wait a few weeks for this, and the other vt patches that
were marked with cc: stable@ before queueing them up.

thanks,

greg k-h
Message details and related content
On Tue, May 18, 2021 at 07:45:59AM +0200, Greg KH wrote:
On Mon, May 17, 2021 at 06:35:24PM -0700, Linus Torvalds wrote:
quoted
On Mon, May 17, 2021 at 6:09 PM Sasha Levin [off-list ref] wrote:
quoted
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

[ Upstream commit ffb324e6f874121f7dce5bdae5e05d02baae7269 ]
So I think the commit is fine, and yes, it should be applied to
stable, but it's one of those "there were three different patches in
as many days to fix the problem, and this is the right one, but maybe
stable should hold off for a while to see that there aren't any
problem reports".

I don't think there will be any problems from this, but while the
patch is tiny, it's conceptually quite a big change to something that
people haven't really touched for a long time.

So use your own judgement, but it might be a good idea to wait a week
before backporting this to see if anything screams.
I was going to wait a few weeks for this, and the other vt patches that
were marked with cc: stable@ before queueing them up.
I'll drop it from my queue then.

-- 
Thanks,
Sasha
Message details and related content
On Tue, May 18, 2021 at 09:22:48AM -0400, Sasha Levin wrote:
On Tue, May 18, 2021 at 07:45:59AM +0200, Greg KH wrote:
quoted
On Mon, May 17, 2021 at 06:35:24PM -0700, Linus Torvalds wrote:
quoted
On Mon, May 17, 2021 at 6:09 PM Sasha Levin [off-list ref] wrote:
quoted
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

[ Upstream commit ffb324e6f874121f7dce5bdae5e05d02baae7269 ]
So I think the commit is fine, and yes, it should be applied to
stable, but it's one of those "there were three different patches in
as many days to fix the problem, and this is the right one, but maybe
stable should hold off for a while to see that there aren't any
problem reports".

I don't think there will be any problems from this, but while the
patch is tiny, it's conceptually quite a big change to something that
people haven't really touched for a long time.

So use your own judgement, but it might be a good idea to wait a week
before backporting this to see if anything screams.
I was going to wait a few weeks for this, and the other vt patches that
were marked with cc: stable@ before queueing them up.
I'll drop it from my queue then.
Thanks!
Message details and related content
On Tue, May 18, 2021 at 07:45:59AM +0200, Greg KH wrote:
On Mon, May 17, 2021 at 06:35:24PM -0700, Linus Torvalds wrote:
quoted
On Mon, May 17, 2021 at 6:09 PM Sasha Levin [off-list ref] wrote:
quoted
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

[ Upstream commit ffb324e6f874121f7dce5bdae5e05d02baae7269 ]
So I think the commit is fine, and yes, it should be applied to
stable, but it's one of those "there were three different patches in
as many days to fix the problem, and this is the right one, but maybe
stable should hold off for a while to see that there aren't any
problem reports".

I don't think there will be any problems from this, but while the
patch is tiny, it's conceptually quite a big change to something that
people haven't really touched for a long time.

So use your own judgement, but it might be a good idea to wait a week
before backporting this to see if anything screams.
I was going to wait a few weeks for this, and the other vt patches that
were marked with cc: stable@ before queueing them up.
I have now queued all of these up.

greg k-h
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help