Re: [PATCH] fbdev: Detect integer underflow at "struct fbcon_ops"->clear_margins.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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]

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux