KASAN: global-out-of-bounds Read in fbcon_resize

KASAN: global-out-of-bounds Read in fbcon_resize

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    8eb858df Add linux-next specific files for 20200819
git tree:       linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x\x1158a00e900000
kernel config:  https://syzkaller.appspot.com/x/.config?xÍ187ef624ef7f02
dashboard link: https://syzkaller.appspot.com/bug?extid³8b1ef6edf0c74a8d97
compiler:       gcc (GCC) 10.1.0-syz 20200507
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x\x146a5589900000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x\x165aa636900000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+b38b1ef6edf0c74a8d97@syzkaller.appspotmail.com

=================================
BUG: KASAN: global-out-of-bounds in fbcon_resize+0x781/0x810 drivers/video/fbdev/core/fbcon.c:2206
Read of size 4 at addr ffffffff8896d418 by task syz-executor732/6868

CPU: 0 PID: 6868 Comm: syz-executor732 Not tainted 5.9.0-rc1-next-20200819-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x18f/0x20d lib/dump_stack.c:118
 print_address_description.constprop.0.cold+0x5/0x497 mm/kasan/report.c:383
 __kasan_report mm/kasan/report.c:513 [inline]
 kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
 fbcon_resize+0x781/0x810 drivers/video/fbdev/core/fbcon.c:2206
 resize_screen drivers/tty/vt/vt.c:1175 [inline]
 vc_do_resize+0x535/0x1150 drivers/tty/vt/vt.c:1246
 vt_ioctl+0x11d2/0x2cc0 drivers/tty/vt/vt_ioctl.c:1025
 tty_ioctl+0x1019/0x15f0 drivers/tty/tty_io.c:2656
 vfs_ioctl fs/ioctl.c:48 [inline]
 __do_sys_ioctl fs/ioctl.c:753 [inline]
 __se_sys_ioctl fs/ioctl.c:739 [inline]
 __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:739
 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x440329
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 db 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc8ff997d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440329
RDX: 0000000020000040 RSI: 0000000000005609 RDI: 0000000000000004
RBP: 00000000006ca018 R08: 000000000000000d R09: 00000000004002c8
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401b90
R13: 0000000000401c20 R14: 0000000000000000 R15: 0000000000000000

The buggy address belongs to the variable:
 font_vga_8x16+0x58/0x60

Memory state around the buggy address:
 ffffffff8896d300: 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9
 ffffffff8896d380: 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 f9 f9 f9
>ffffffff8896d400: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
                            ^
 ffffffff8896d480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ffffffff8896d500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=================================


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

[PATCH] fbcon: Fix user font detection test at fbcon_resize().

[Tetsuo Handa]

syzbot is reporting OOB read at fbcon_resize() [1], for
commit 39b3cffb8cf31117 ("fbcon: prevent user font height or width change
 from causing potential out-of-bounds access") is by error using
registered_fb[con2fb_map[vc->vc_num]]->fbcon_par->p->userfont (which was
set to non-zero) instead of fb_display[vc->vc_num].userfont (which remains
zero for that display).

We could remove tricky userfont flag [2], for we can determine it by
comparing address of the font data and addresses of built-in font data.
But since that commit is failing to fix the original OOB read [3], this
patch keeps the change minimal in case we decide to revert altogether.

[1] https://syzkaller.appspot.com/bug?idëcbbb6576958a496500fee9cf7aa83ea00b5920
[2] https://syzkaller.appspot.com/text?tag=Patch&x\x14030853900000
[3] https://syzkaller.appspot.com/bug?idoba8c186d97cf1011ab17660e633b1cc4e080c9

Reported-by: syzbot <syzbot+b38b1ef6edf0c74a8d97@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: 39b3cffb8cf31117 ("fbcon: prevent user font height or width change from causing potential out-of-bounds access")
Cc: George Kennedy <george.kennedy@oracle.com>
---
 drivers/video/fbdev/core/fbcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 66167830fefd..dae7ae7f225a 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2203,7 +2203,7 @@ static int fbcon_resize(struct vc_data *vc, unsigned int width,
 	struct fb_var_screeninfo var = info->var;
 	int x_diff, y_diff, virt_w, virt_h, virt_fw, virt_fh;
 
-	if (ops->p && ops->p->userfont && FNTSIZE(vc->vc_font.data)) {
+	if (p->userfont && FNTSIZE(vc->vc_font.data)) {
 		int size;
 		int pitch = PITCH(vc->vc_font.width);
 
-- 
2.18.4

Re: [PATCH] fbcon: Fix user font detection test at fbcon_resize().

[Tetsuo Handa]

Greg, will you pick up this patch?

It seems that finding the real cause of [3] and actually fixing [3] will be difficult.
Since I can't reproduce [3] locally, I will have to try flood of "#syz test" requests
for debug printk() patches.

On 2020/09/11 7:57, Tetsuo Handa wrote:
> syzbot is reporting OOB read at fbcon_resize() [1], for
> commit 39b3cffb8cf31117 ("fbcon: prevent user font height or width change
>  from causing potential out-of-bounds access") is by error using
> registered_fb[con2fb_map[vc->vc_num]]->fbcon_par->p->userfont (which was
> set to non-zero) instead of fb_display[vc->vc_num].userfont (which remains
> zero for that display).
> 
> We could remove tricky userfont flag [2], for we can determine it by
> comparing address of the font data and addresses of built-in font data.
> But since that commit is failing to fix the original OOB read [3], this
> patch keeps the change minimal in case we decide to revert altogether.
> 
> [1] https://syzkaller.appspot.com/bug?idëcbbb6576958a496500fee9cf7aa83ea00b5920
> [2] https://syzkaller.appspot.com/text?tag=Patch&x\x14030853900000
> [3] https://syzkaller.appspot.com/bug?idoba8c186d97cf1011ab17660e633b1cc4e080c9
> 
> Reported-by: syzbot <syzbot+b38b1ef6edf0c74a8d97@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Fixes: 39b3cffb8cf31117 ("fbcon: prevent user font height or width change from causing potential out-of-bounds access")
> Cc: George Kennedy <george.kennedy@oracle.com>
> ---
>  drivers/video/fbdev/core/fbcon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 66167830fefd..dae7ae7f225a 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -2203,7 +2203,7 @@ static int fbcon_resize(struct vc_data *vc, unsigned int width,
>  	struct fb_var_screeninfo var = info->var;
>  	int x_diff, y_diff, virt_w, virt_h, virt_fw, virt_fh;
>  
> -	if (ops->p && ops->p->userfont && FNTSIZE(vc->vc_font.data)) {
> +	if (p->userfont && FNTSIZE(vc->vc_font.data)) {
>  		int size;
>  		int pitch = PITCH(vc->vc_font.width);
>  
> 

Re: [PATCH] fbcon: Fix user font detection test at fbcon_resize().

[Daniel Vetter]

On Wed, Sep 16, 2020 at 09:01:06AM +0900, Tetsuo Handa wrote:
> Greg, will you pick up this patch?
> 
> It seems that finding the real cause of [3] and actually fixing [3] will be difficult.
> Since I can't reproduce [3] locally, I will have to try flood of "#syz test" requests
> for debug printk() patches.

Grasping for straws, but have you retested with the scrollback code
removed already? That was full of fail and we outright deleted it:

50145474f6ef ("fbcon: remove soft scrollback code")

Cheers, Daniel

> 
> On 2020/09/11 7:57, Tetsuo Handa wrote:
> > syzbot is reporting OOB read at fbcon_resize() [1], for
> > commit 39b3cffb8cf31117 ("fbcon: prevent user font height or width change
> >  from causing potential out-of-bounds access") is by error using
> > registered_fb[con2fb_map[vc->vc_num]]->fbcon_par->p->userfont (which was
> > set to non-zero) instead of fb_display[vc->vc_num].userfont (which remains
> > zero for that display).
> > 
> > We could remove tricky userfont flag [2], for we can determine it by
> > comparing address of the font data and addresses of built-in font data.
> > But since that commit is failing to fix the original OOB read [3], this
> > patch keeps the change minimal in case we decide to revert altogether.
> > 
> > [1] https://syzkaller.appspot.com/bug?idëcbbb6576958a496500fee9cf7aa83ea00b5920
> > [2] https://syzkaller.appspot.com/text?tag=Patch&x\x14030853900000
> > [3] https://syzkaller.appspot.com/bug?idoba8c186d97cf1011ab17660e633b1cc4e080c9
> > 
> > Reported-by: syzbot <syzbot+b38b1ef6edf0c74a8d97@syzkaller.appspotmail.com>
> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > Fixes: 39b3cffb8cf31117 ("fbcon: prevent user font height or width change from causing potential out-of-bounds access")
> > Cc: George Kennedy <george.kennedy@oracle.com>
> > ---
> >  drivers/video/fbdev/core/fbcon.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> > index 66167830fefd..dae7ae7f225a 100644
> > --- a/drivers/video/fbdev/core/fbcon.c
> > +++ b/drivers/video/fbdev/core/fbcon.c
> > @@ -2203,7 +2203,7 @@ static int fbcon_resize(struct vc_data *vc, unsigned int width,
> >  	struct fb_var_screeninfo var = info->var;
> >  	int x_diff, y_diff, virt_w, virt_h, virt_fw, virt_fh;
> >  
> > -	if (ops->p && ops->p->userfont && FNTSIZE(vc->vc_font.data)) {
> > +	if (p->userfont && FNTSIZE(vc->vc_font.data)) {
> >  		int size;
> >  		int pitch = PITCH(vc->vc_font.width);
> >  
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] fbcon: Fix user font detection test at fbcon_resize().

[Greg KH]

On Wed, Sep 16, 2020 at 09:01:06AM +0900, Tetsuo Handa wrote:
> Greg, will you pick up this patch?
> 
> It seems that finding the real cause of [3] and actually fixing [3] will be difficult.
> Since I can't reproduce [3] locally, I will have to try flood of "#syz test" requests
> for debug printk() patches.

I agree with Daniel here, can you retest these against Linus's latest
tree please?

thanks,

greg k-h

Re: [PATCH] fbcon: Fix user font detection test at fbcon_resize().

[Tetsuo Handa]

On 2020/09/16 17:26, Greg KH wrote:
> On Wed, Sep 16, 2020 at 09:01:06AM +0900, Tetsuo Handa wrote:
>> Greg, will you pick up this patch?
>>
>> It seems that finding the real cause of [3] and actually fixing [3] will be difficult.
>> Since I can't reproduce [3] locally, I will have to try flood of "#syz test" requests
>> for debug printk() patches.
> 
> I agree with Daniel here, can you retest these against Linus's latest
> tree please?
> 

syzbot already reproduced these bugs using the latest commit. ;-)

You can find

  ci-upstream-kasan-gce-root 	2020/09/15 15:18 	upstream 	fc4f28bb 

record for "KASAN: global-out-of-bounds Read in bit_putcs" and

  ci-upstream-kasan-gce-root 	2020/09/16 09:54 	upstream 	fc4f28bb

record for "KASAN: global-out-of-bounds Read in fbcon_resize".

Re: [PATCH] fbcon: Fix user font detection test at fbcon_resize().

[Greg KH]

On Wed, Sep 16, 2020 at 07:06:31PM +0900, Tetsuo Handa wrote:
> On 2020/09/16 17:26, Greg KH wrote:
> > On Wed, Sep 16, 2020 at 09:01:06AM +0900, Tetsuo Handa wrote:
> >> Greg, will you pick up this patch?
> >>
> >> It seems that finding the real cause of [3] and actually fixing [3] will be difficult.
> >> Since I can't reproduce [3] locally, I will have to try flood of "#syz test" requests
> >> for debug printk() patches.
> > 
> > I agree with Daniel here, can you retest these against Linus's latest
> > tree please?
> > 
> 
> syzbot already reproduced these bugs using the latest commit. ;-)
> 
> You can find
> 
>   ci-upstream-kasan-gce-root 	2020/09/15 15:18 	upstream 	fc4f28bb 
> 
> record for "KASAN: global-out-of-bounds Read in bit_putcs" and
> 
>   ci-upstream-kasan-gce-root 	2020/09/16 09:54 	upstream 	fc4f28bb
> 
> record for "KASAN: global-out-of-bounds Read in fbcon_resize".

Ok, will pick it up now, thanks.

greg k-h