| Message ID | f6e3e611-8704-1263-d163-f52c906a4f06@I-love.SAKURA.ne.jp (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | fbcon: Fix user font detection test at fbcon_resize(). | expand |
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=ebcbbb6576958a496500fee9cf7aa83ea00b5920 > [2] https://syzkaller.appspot.com/text?tag=Patch&x=14030853900000 > [3] https://syzkaller.appspot.com/bug?id=6fba8c186d97cf1011ab17660e633b1cc4e080c9 > > 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); > >
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=ebcbbb6576958a496500fee9cf7aa83ea00b5920 > > [2] https://syzkaller.appspot.com/text?tag=Patch&x=14030853900000 > > [3] https://syzkaller.appspot.com/bug?id=6fba8c186d97cf1011ab17660e633b1cc4e080c9 > > > > 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); > > > > >
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
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".
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
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);
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=ebcbbb6576958a496500fee9cf7aa83ea00b5920 [2] https://syzkaller.appspot.com/text?tag=Patch&x=14030853900000 [3] https://syzkaller.appspot.com/bug?id=6fba8c186d97cf1011ab17660e633b1cc4e080c9 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(-)