[PATCH AUTOSEL 5.12 5/5] tty: vt: always invoke vc->vc_sw->con_resize callback

[PATCH AUTOSEL 5.12 5/5] tty: vt: always invoke vc->vc_sw->con_resize callback

[Sasha Levin]

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 <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>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 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 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

Re: [PATCH AUTOSEL 5.12 5/5] tty: vt: always invoke vc->vc_sw->con_resize callback

[Linus Torvalds]

On Mon, May 17, 2021 at 6:09 PM Sasha Levin <sashal@kernel.org> 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

Re: [PATCH AUTOSEL 5.12 5/5] tty: vt: always invoke vc->vc_sw->con_resize callback

[Greg KH]

On Mon, May 17, 2021 at 06:35:24PM -0700, Linus Torvalds wrote:
> On Mon, May 17, 2021 at 6:09 PM Sasha Levin <sashal@kernel.org> 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.

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

Re: [PATCH AUTOSEL 5.12 5/5] tty: vt: always invoke vc->vc_sw->con_resize callback

[Sasha Levin]

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:
>> On Mon, May 17, 2021 at 6:09 PM Sasha Levin <sashal@kernel.org> 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.
>
>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

Re: [PATCH AUTOSEL 5.12 5/5] tty: vt: always invoke vc->vc_sw->con_resize callback

[Greg KH]

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:
> > On Mon, May 17, 2021 at 06:35:24PM -0700, Linus Torvalds wrote:
> > > On Mon, May 17, 2021 at 6:09 PM Sasha Levin <sashal@kernel.org> 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.
> > 
> > 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!

Re: [PATCH AUTOSEL 5.12 5/5] tty: vt: always invoke vc->vc_sw->con_resize callback

[Greg KH]

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:
> > On Mon, May 17, 2021 at 6:09 PM Sasha Levin <sashal@kernel.org> 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.
> 
> 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