From: Dan Carpenter <dan.carpenter@oracle.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Randy Dunlap <rdunlap@infradead.org>,
syzbot <syzbot+04168c8063cfdde1db5e@syzkaller.appspotmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Colin King <colin.king@canonical.com>,
DRI Development <dri-devel@lists.freedesktop.org>,
Linux Fbdev development list <linux-fbdev@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Masahiro Yamada <masahiroy@kernel.org>,
syzkaller-bugs@googlegroups.com
Subject: Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
Date: Mon, 30 Aug 2021 16:00:00 +0300 [thread overview]
Message-ID: <20210830130000.GW7722@kadam> (raw)
In-Reply-To: <CAMuHMdV=xVhEHLEoYt3OF+kmGrLOr6t7SP1sghSmp9JqXD+3Og@mail.gmail.com>
On Mon, Aug 30, 2021 at 02:00:21PM +0200, Geert Uytterhoeven wrote:
> Hi Testsuo,
>
> On Mon, Aug 30, 2021 at 4:27 AM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
> > On 2021/08/30 9:24, Randy Dunlap wrote:
> > > Note that yres_virtual is set to 0x10000000. Is there no practical limit
> > > (hence limit check) that can be used here?
> > >
> > > Also, in vga16fb_check_var(), beginning at line 404:
> > >
> > > 404 if (yres > vyres)
> > > 405 vyres = yres;
> > > 406 if (vxres * vyres > maxmem) {
> > > 407 vyres = maxmem / vxres;
> > > 408 if (vyres < yres)
> > > 409 return -ENOMEM;
> > > 410 }
> > >
> > > At line 406, the product of vxres * vyres overflows 32 bits (is 0 in this
> > > case/example), so any protection from this block is lost.
> >
> > OK. Then, we can check overflow like below.
> >
> > diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
> > index e2757ff1c23d..e483a3f5fd47 100644
> > --- a/drivers/video/fbdev/vga16fb.c
> > +++ b/drivers/video/fbdev/vga16fb.c
> > @@ -403,7 +403,7 @@ static int vga16fb_check_var(struct fb_var_screeninfo *var,
> >
> > if (yres > vyres)
> > vyres = yres;
> > - if (vxres * vyres > maxmem) {
> > + if ((u64) vxres * vyres > (u64) maxmem) {
>
> Mindlessly changing the sizes is not the solution.
> Please use e.g. the array_size() helper from <linux/overflow.h>
> instead.
On a 64bit system the array_size() macro is going to do the exact same
casts? But I do think this code would be easier to understand if the
integer overflow check were pull out separately and done first:
if (array_size(vxres, vyres) >= UINT_MAX)
return -EINVAL;
if (vxres * vyres > maxmem) {
...
The UINT_MAX is because vxres and vyres are u32.
This would maybe be the first time anyone ever did an integer overflow
check like this in the kernel. It's a new idiom.
regards,
dan carpenter
next prev parent reply other threads:[~2021-08-30 13:00 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-14 5:16 [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect syzbot
2021-07-14 5:16 ` syzbot
2021-08-30 0:24 ` Randy Dunlap
2021-08-30 2:27 ` Tetsuo Handa
2021-08-30 2:31 ` Randy Dunlap
2021-08-30 16:05 ` [PATCH] fbmem: don't allow too huge resolutions Tetsuo Handa
2021-08-31 6:48 ` Geert Uytterhoeven
2021-08-31 6:48 ` Geert Uytterhoeven
2021-08-31 15:23 ` Tetsuo Handa
2021-08-31 16:20 ` Daniel Vetter
2021-08-31 16:20 ` Daniel Vetter
2021-08-31 17:19 ` Geert Uytterhoeven
2021-08-31 17:19 ` Geert Uytterhoeven
2021-08-31 18:53 ` Daniel Vetter
2021-08-31 18:53 ` Daniel Vetter
2021-08-31 18:56 ` Geert Uytterhoeven
2021-08-31 18:56 ` Geert Uytterhoeven
2021-08-31 19:04 ` Daniel Vetter
2021-08-31 19:04 ` Daniel Vetter
2021-09-01 1:14 ` [PATCH v2] " Tetsuo Handa
2021-09-01 7:12 ` Geert Uytterhoeven
2021-09-01 7:12 ` Geert Uytterhoeven
2021-09-08 10:27 ` [PATCH v2 (repost)] " Tetsuo Handa
2021-09-08 16:52 ` Daniel Vetter
2021-08-30 12:00 ` [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect Geert Uytterhoeven
2021-08-30 12:00 ` Geert Uytterhoeven
2021-08-30 13:00 ` Dan Carpenter [this message]
2021-08-30 13:37 ` Tetsuo Handa
2021-08-30 13:47 ` Dan Carpenter
2021-08-30 14:25 ` Tetsuo Handa
2021-08-30 14:29 ` Dan Carpenter
2021-08-30 14:30 ` Geert Uytterhoeven
2021-08-30 14:30 ` Geert Uytterhoeven
2021-08-30 14:38 ` Tetsuo Handa
2021-08-30 14:53 ` Geert Uytterhoeven
2021-08-30 14:53 ` Geert Uytterhoeven
2021-08-30 15:00 ` Geert Uytterhoeven
2021-08-30 15:00 ` Geert Uytterhoeven
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210830130000.GW7722@kadam \
--to=dan.carpenter@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=b.zolnierkie@samsung.com \
--cc=colin.king@canonical.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=geert@linux-m68k.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=masahiroy@kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=rdunlap@infradead.org \
--cc=syzbot+04168c8063cfdde1db5e@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.