Re: [PATCH V4] fbcon: fix fbcon out-of-bounds write in sys_imageblit
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-08-30 14:59:33
Also in:
dri-devel, lkml
Hi Tetsuo, On Mon, Aug 30, 2021 at 12:25 PM Tetsuo Handa [off-list ref] wrote:
On 2021/08/30 17:16, Daniel Vetter wrote:quoted
On Mon, Aug 30, 2021 at 11:30:23AM +0800, tcs.kernel@gmail.com wrote:quoted
From: Haimin Zhang <redacted> yres and vyres can be controlled by user mode parameters, and cause p->vrows to become a negative value. While this value be passed to real_y function, the ypos will be out of screen range.This is an out-of-bounds write bug. some driver will check xres and yres in fb_check_var callback,but some not so we add a common check after that callback. Signed-off-by: Haimin Zhang <redacted> Signed-off-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>Please s/Signed-off-by: Tetsuo Handa/Suggested-by: Tetsuo Handa/ . It is Haimin who debugged this problem and wrote this patch.quoted
Does this fix a syzbot crash or how was this discovered?Yes, Haimin's team is running syzkaller locally and found this bug. Therefore, no Reported-by: syzbot tag for this patch. -------- Forwarded Message -------- Subject: Re: [PATCH v2] fbcon: fix Out-Of-Bounds write in sys_imageblit Message-ID: [off-list ref] Date: Mon, 2 Aug 2021 14:50:24 +0800 hi, Tetsuo Handa i made a test with your suggested code, it can block the out-of-bound bug. where to add this check logic, i suggest to add it after the driver's fb_check_var callback.because what we plan to add is a common check by framework,but some driver can fault-tolerant invalid parameters(like yres_virtual > yres) /* exist common check */ if (var->xres < 8 || var->yres < 8) return -EINVAL; /* callback to drivers, some driver can fix invalid virtual xres or virtual yres */
Yes. Fbdev drivers are supposed to round up invalid or unsupported values, or return -EINVAL.
quoted hunk ↗ jump to hunk
ret = info->fbops->fb_check_var(var, info); if (ret) return ret; /* we add a more check here, if some drivers can't fix invalid x,y virtual values, we return a -EINVAL */ if (var->yres_virtual < var->yres || var->xres_virtual < var->xres) return -EINVAL; how about this fix ? i can make a v3 patch.diff --git a/drivers/video/fbdev/core/fbmem.cb/drivers/video/fbdev/core/fbmem.c index 1c85514..9fb7e94 100644--- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c@@ -1012,6 +1012,10 @@ static int fb_check_caps(struct fb_info *info,struct fb_var_screeninfo *var, if (ret) return ret; + + /* Virtual resolution cannot be smaller than visible resolution. */ + if (var->yres_virtual < var->yres || var->xres_virtual < var->xres) + return -EINVAL;
So if this happens, it's a driver bug, not a userspace bug.
if ((var->activate & FB_ACTIVATE_MASK) != FB_ACTIVATE_NOW)
return 0;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds