Hi Tetsuo,
Thanks for your patch!
On Mon, Aug 30, 2021 at 6:05 PM Tetsuo Handa penguin-kernel@i-love.sakura.ne.jp wrote:
syzbot is reporting page fault at vga16fb_fillrect() [1], for vga16fb_check_var() is failing to detect multiplication overflow.
if (vxres * vyres > maxmem) { vyres = maxmem / vxres; if (vyres < yres) return -ENOMEM; }
IMHO that should be fixed in vga16fb, too.
Since no module would accept too huge resolutions where multiplication overflow happens, let's reject in the common path.
This patch does not use array_size(), for array_size() is allowed to return UINT_MAX on 32bits even if overflow did not happen. We want to detect only overflow here, for individual module will recheck with more strict limits as needed.
Which is IMHO not really an issue, as I believe on 32-bit you cannot use a very large frame buffer, long before you reach UINT_MAX.
Link: https://syzkaller.appspot.com/bug?extid=04168c8063cfdde1db5e [1] Reported-by: syzbot syzbot+04168c8063cfdde1db5e@syzkaller.appspotmail.com Debugged-by: Randy Dunlap rdunlap@infradead.org Signed-off-by: Tetsuo Handa penguin-kernel@I-love.SAKURA.ne.jp Tested-by: syzbot syzbot+04168c8063cfdde1db5e@syzkaller.appspotmail.com
drivers/video/fbdev/core/fbmem.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 1c855145711b..9f5075dc2345 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1008,6 +1008,11 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var) if (var->xres < 8 || var->yres < 8) return -EINVAL;
/* Don't allow u32 * u32 to overflow. */if ((u64) var->xres * var->yres > UINT_MAX ||(u64) var->xres_virtual * var->yres_virtual > UINT_MAX)return -EINVAL;
I think it would still be better to use check_mul_overflow(), as that makes it clear and explicit what is being done, even without a comment.
Furthermore, this restricts the virtual frame buffer size on 64-bit, too, while graphics cards can have much more than 4 GiB of RAM.
ret = info->fbops->fb_check_var(var, info); if (ret)
Gr{oetje,eeting}s,
Geert