Re: [PATCH V4] fbcon: fix fbcon out-of-bounds write in sys_imageblit

Hi Tetsuo,

On Mon, Aug 30, 2021 at 12:25 PM Tetsuo Handa penguin-kernel@i-love.sakura.ne.jp wrote:

On 2021/08/30 17:16, Daniel Vetter wrote:

...

On Mon, Aug 30, 2021 at 11:30:23AM +0800, tcs.kernel@gmail.com wrote:

...

From: Haimin Zhang tcs_kernel@tencent.com

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 tcs_kernel@tencent.com 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.

...

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: 33fc0e30-b94c-939f-a708-4b939af43100@gmail.com 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.

    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.c b/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