From: George Kennedy <george.kennedy@oracle.com>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: linux-fbdev@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
Dan Carpenter <dan.carpenter@oracle.com>,
Jiri Slaby <jslaby@suse.com>, Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [PATCH] fbdev: Detect integer underflow at "struct fbcon_ops"->clear_margins.
Date: Tue, 14 Jul 2020 13:15:53 -0400 [thread overview]
Message-ID: <c5bf6d5c-8d0a-8df5-2a11-38bf37a11d67@oracle.com> (raw)
In-Reply-To: <e00078d1-e5fb-a019-3036-cb182ed2e40b@i-love.sakura.ne.jp>
[-- Attachment #1.1: Type: text/plain, Size: 9093 bytes --]
Hello Tetsuo,
Can you try the a.out built from the original Syzkaller modified repro C
program? It walks 0-7 through xres and yres of the fb_var_screeninfo struct.
// https://syzkaller.appspot.com/bug?id=a565882df74fa76f10d3a6fec4be31098dbb37c6
// autogenerated by syzkaller (https://github.com/google/syzkaller)
#define _GNU_SOURCE
#include <endian.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <sys/fcntl.h>
#include <unistd.h>
#include <errno.h>
#include <linux/fb.h>
int verbose = 0;
void
dumpit(unsigned char *buf, int count, int addr)
{
int i, j;
char bp[256];
memset(bp, 0, 256);
for (i = j = 0; i < count; i++, j++) {
if (j == 16) {
j = 0;
printf("%s\n", bp);
memset(bp, 0, 256);
}
if (j == 0) {
sprintf(&bp[strlen(bp)], "%x: ", addr + i);
}
sprintf(&bp[strlen(bp)], "%02x ", buf[i]);
}
if (j != 0) {
printf("%s\n", bp);
}
}
uint64_t r[1] = {0xffffffffffffffff};
int main(int argc, char **argv)
{
syscall(__NR_mmap, 0x20000000ul, 0x1000000ul, 3ul, 0x32ul, -1, 0);
intptr_t res = 0;
uint32_t activate = FB_ACTIVATE_NOW;
struct fb_var_screeninfo *varp = (struct fb_var_screeninfo *)0x200001c0;
struct fb_var_screeninfo *starting_varp = malloc(sizeof(struct fb_var_screeninfo *));
char *vp = (char *)varp;
int i, sum, rtn, c;
extern char *optarg;
int limit = 0, passes = 0;
unsigned int start_address = 0;
unsigned int pattern = 0;
int breakit = 1;
while ((c = getopt (argc, argv, "a:v")) != -1)
switch (c)
{
case 'a':
activate = strtol(optarg, 0, 0);
break;
case 'v':
verbose++;
break;
default:
fprintf(stderr, "\nusage: %s [-a <activate code>] [-v]\n\n", argv[0]);
return -1;
}
int fd = open("/dev/fb0", O_RDWR);
if (fd < 0) {
perror("open");
return 0;
}
printf("fd: %d\n", fd);
r[0] = fd;
rtn = syscall(__NR_ioctl, r[0], 0x4600ul, 0x200001c0ul);
if (rtn < 0) {
perror("ioctl");
fprintf(stderr, "rtn=%d, errno=%d\n", rtn, errno);
}
if (verbose) {
printf("FBIOGET_VSCREENINFO:\n");
dumpit((unsigned char *)vp, sizeof(struct fb_var_screeninfo), 0x200001c0);
}
memcpy(starting_varp, varp, sizeof(struct fb_var_screeninfo));
fprintf(stderr, "activate = %d\n", activate);
varp->activate = activate;
if (verbose) {
printf("Pre FBIOPUT_VSCREENINFO:\n");
dumpit((unsigned char *)vp, sizeof(struct fb_var_screeninfo), 0x200001c0);
sleep(2);
}
rtn = syscall(__NR_ioctl, r[0], 0x4601ul, 0x200001c0ul);
if (rtn < 0) {
perror("ioctl");
fprintf(stderr, "rtn=%d, errno=%d\n", rtn, errno);
}
limit = 2;
for (pattern = 0 ; pattern < 8 ; pattern++) {
unsigned long addr = 0x200001c0;
passes = 0;
printf("\nWalk START addr = 0x%x, Break pattern=%x\n", addr, pattern);
while (addr <= 0x2000025c) {
fprintf(stderr, "======================== %d: addr=%x ========================\n", passes, addr);
memcpy(varp, starting_varp, sizeof(struct fb_var_screeninfo));
*(uint32_t*)addr = pattern;
varp->activate = activate;
printf("Pre FBIOPUT_VSCREENINFO: pattern=%x\n", pattern);
dumpit((unsigned char *)vp, sizeof(struct fb_var_screeninfo), 0x200001c0);
sleep(3);
rtn = syscall(__NR_ioctl, r[0], 0x4601ul, 0x200001c0ul);
if (rtn < 0) {
perror("ioctl");
fprintf(stderr, "rtn=%d, errno=%d\n", rtn, errno);
}
addr += 4;
passes++;
if (passes == limit)
break;
}
}
close(fd);
return 0;
}
With my patch it gets output like the following:
[root@localhost ~]# ./fb_break
fd: 3
activate = 0
Walk START addr = 0x200001c0, Break pattern=0
======================== 0: addr=200001c0 ========================
Pre FBIOPUT_VSCREENINFO: pattern=0
200001c0: 00 00 00 00 00 03 00 00 00 04 00 00 00 03 00 00
200001d0: 00 00 00 00 00 00 00 00 20 00 00 00 00 00 00 00
200001e0: 10 00 00 00 08 00 00 00 00 00 00 00 08 00 00 00
200001f0: 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 00
20000200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000210: 00 00 00 00 00 00 00 00 2c 01 00 00 90 01 00 00
20000220: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000230: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000240: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000250: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ioctl: Invalid argument
rtn=-1, errno=22
======================== 1: addr=200001c4 ========================
Pre FBIOPUT_VSCREENINFO: pattern=0
200001c0: 00 04 00 00 00 00 00 00 00 04 00 00 00 03 00 00
200001d0: 00 00 00 00 00 00 00 00 20 00 00 00 00 00 00 00
200001e0: 10 00 00 00 08 00 00 00 00 00 00 00 08 00 00 00
200001f0: 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 00
20000200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000210: 00 00 00 00 00 00 00 00 2c 01 00 00 90 01 00 00
20000220: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000230: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000240: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000250: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ioctl: Invalid argument
rtn=-1, errno=22
Walk START addr = 0x200001c0, Break pattern=1
======================== 0: addr=200001c0 ========================
Pre FBIOPUT_VSCREENINFO: pattern=1
200001c0: 01 00 00 00 00 03 00 00 00 04 00 00 00 03 00 00
200001d0: 00 00 00 00 00 00 00 00 20 00 00 00 00 00 00 00
200001e0: 10 00 00 00 08 00 00 00 00 00 00 00 08 00 00 00
200001f0: 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 00
20000200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000210: 00 00 00 00 00 00 00 00 2c 01 00 00 90 01 00 00
20000220: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000230: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000240: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000250: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ioctl: Invalid argument
rtn=-1, errno=22
...
======================== 1: addr=200001c4 ========================
Pre FBIOPUT_VSCREENINFO: pattern=7
200001c0: 00 04 00 00 07 00 00 00 00 04 00 00 00 03 00 00
200001d0: 00 00 00 00 00 00 00 00 20 00 00 00 00 00 00 00
200001e0: 10 00 00 00 08 00 00 00 00 00 00 00 08 00 00 00
200001f0: 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 00
20000200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000210: 00 00 00 00 00 00 00 00 2c 01 00 00 90 01 00 00
20000220: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000230: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000240: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20000250: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ioctl: Invalid argument
rtn=-1, errno=22
[root@localhost ~]#
Thank you,
George
On 7/14/2020 6:27 AM, Tetsuo Handa wrote:
> On 2020/07/14 16:22, Bartlomiej Zolnierkiewicz wrote:
>> How does this patch relate to:
>>
>> https://marc.info/?l=linux-fbdev&m=159415024816722&w=2
>>
>> ?
>>
>> It seems to address the same issue, I've added George and Dan to Cc:.
> George Kennedy's patch does not help for my case.
>
> You can try a.out built from
>
> ----------
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <sys/ioctl.h>
> #include <linux/fb.h>
>
> int main(int argc, char *argv[])
> {
> const int fd = open("/dev/fb0", O_ACCMODE);
> struct fb_var_screeninfo var = { };
> ioctl(fd, FBIOGET_VSCREENINFO, &var);
> var.xres = var.yres = 16;
> ioctl(fd, FBIOPUT_VSCREENINFO, &var);
> return 0;
> }
> ----------
>
> with a fault injection patch
>
> ----------
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -1214,6 +1214,10 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
>
> if (new_screen_size > KMALLOC_MAX_SIZE)
> return -EINVAL;
> + if (!strcmp(current->comm, "a.out")) {
> + printk(KERN_INFO "Forcing memory allocation failure.\n");
> + return -ENOMEM;
> + }
> newscreen = kzalloc(new_screen_size, GFP_USER);
> if (!newscreen)
> return -ENOMEM;
> ----------
>
> . What my patch workarounds is cases when vc_do_resize() did not update vc->vc_{cols,rows} .
> Unless vc->vc_{cols,rows} are updated by vc_do_resize() in a way that avoids integer underflow at
>
> unsigned int rw = info->var.xres - (vc->vc_cols*cw);
> unsigned int bh = info->var.yres - (vc->vc_rows*ch);
>
> , this crash won't go away.
>
> [ 39.995757][ T2788] Forcing memory allocation failure.
> [ 39.996527][ T2788] BUG: unable to handle page fault for address: ffffa9d180d7b000
> [ 39.996529][ T2788] #PF: supervisor write access in kernel mode
> [ 39.996530][ T2788] #PF: error_code(0x0002) - not-present page
> [ 39.996531][ T2788] PGD 13a48c067 P4D 13a48c067 PUD 13a48d067 PMD 1324e4067 PTE 0
> [ 39.996547][ T2788] Oops: 0002 [#1] SMP
> [ 39.996550][ T2788] CPU: 2 PID: 2788 Comm: a.out Not tainted 5.8.0-rc5+ #757
> [ 39.996551][ T2788] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 02/27/2020
> [ 39.996555][ T2788] RIP: 0010:bitfill_aligned+0x87/0x120 [cfbfillrect]
[-- Attachment #1.2: Type: text/html, Size: 10053 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-07-15 7:00 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200710055329.3759-1-penguin-kernel@I-love.SAKURA.ne.jp>
2020-07-10 5:56 ` fbconsole needs more parameter validations Tetsuo Handa
2020-07-10 10:56 ` Greg Kroah-Hartman
2020-07-11 6:16 ` Tetsuo Handa
2020-07-11 11:08 ` Tetsuo Handa
2020-07-12 11:10 ` [PATCH v3] vt: Reject zero-sized screen buffer size Tetsuo Handa
2020-07-12 11:10 ` [PATCH] fbdev: Detect integer underflow at "struct fbcon_ops"->clear_margins Tetsuo Handa
[not found] ` <CGME20200714072231eucas1p17c53f0a661346ebfd316ebd5796ca346@eucas1p1.samsung.com>
2020-07-14 7:22 ` Bartlomiej Zolnierkiewicz
2020-07-14 10:27 ` Tetsuo Handa
2020-07-14 13:37 ` Tetsuo Handa
2020-07-15 1:51 ` [PATCH v2] " Tetsuo Handa
2020-07-15 9:48 ` Dan Carpenter
2020-07-15 11:17 ` Tetsuo Handa
2020-07-15 14:02 ` Tetsuo Handa
2020-07-15 15:12 ` Dan Carpenter
2020-07-15 15:29 ` Tetsuo Handa
2020-07-16 10:00 ` Daniel Vetter
2020-07-16 11:27 ` Tetsuo Handa
2020-07-21 16:08 ` Greg Kroah-Hartman
2020-07-22 8:07 ` Daniel Vetter
2020-07-23 14:21 ` Greg Kroah-Hartman
2020-07-24 8:28 ` Bartlomiej Zolnierkiewicz
2020-07-14 17:15 ` George Kennedy [this message]
2020-07-15 0:24 ` [PATCH] " Tetsuo Handa
2020-08-19 22:07 ` [PATCH v3] vt: Reject zero-sized screen buffer size Kees Cook
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=c5bf6d5c-8d0a-8df5-2a11-38bf37a11d67@oracle.com \
--to=george.kennedy@oracle.com \
--cc=b.zolnierkie@samsung.com \
--cc=dan.carpenter@oracle.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=dvyukov@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.com \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).