From: Haimin Zhang <[email protected]>
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 <[email protected]>
Signed-off-by: Tetsuo Handa <[email protected]>
---
drivers/video/fbdev/core/fbmem.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 1c85514..5599372 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1013,6 +1013,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;
+
if ((var->activate & FB_ACTIVATE_MASK) != FB_ACTIVATE_NOW)
return 0;
--
1.8.3.1
On Mon, Aug 30, 2021 at 11:30:23AM +0800, [email protected] wrote:
> From: Haimin Zhang <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Tetsuo Handa <[email protected]>
> ---
> drivers/video/fbdev/core/fbmem.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 1c85514..5599372 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1013,6 +1013,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;
> +
> if ((var->activate & FB_ACTIVATE_MASK) != FB_ACTIVATE_NOW)
> return 0;
>
> --
> 1.8.3.1
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for what needs to be done
here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
On Mon, Aug 30, 2021 at 11:30:23AM +0800, [email protected] wrote:
> From: Haimin Zhang <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Tetsuo Handa <[email protected]>
Does this fix a syzbot crash or how was this discovered?
-Daniel
> ---
> drivers/video/fbdev/core/fbmem.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 1c85514..5599372 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1013,6 +1013,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;
> +
> if ((var->activate & FB_ACTIVATE_MASK) != FB_ACTIVATE_NOW)
> return 0;
>
> --
> 1.8.3.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On 2021/08/30 17:16, Daniel Vetter wrote:
> On Mon, Aug 30, 2021 at 11:30:23AM +0800, [email protected] wrote:
>> From: Haimin Zhang <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Tetsuo Handa <[email protected]>
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: <[email protected]>
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 */
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;
if ((var->activate & FB_ACTIVATE_MASK) != FB_ACTIVATE_NOW)
return 0;
-------- Forwarded Message --------
Subject: Re: [PATCH]fbcon:fix Out-Of-Bounds write in sys_imageblit
Message-ID: <[email protected]>
Date: Wed, 28 Jul 2021 11:10:52 +0800
this is my debug log:
1. environment
linux kernel: git checkout a4c30b8691f26c6115db6e11ec837c1fb6073953
qemu version:QEMU emulator version 6.0.90 (v6.1.0-rc0-99-g76bf66b913)
qemu-system-x86_64 -smp 4 -kernel
linux_allyes/build/arch/x86/boot/bzImage -m 4G \
-hda /data/h_image/linux_debug/buster1.img -serial stdio \
-append "earlyprintk=serial console=ttyS0 root=/dev/sda nokaslr" \
-enable-kvm -cpu host -display none -net nic -net
user,hostfwd=tcp::1236-:22 -s -S
1.kasan report:
==================================================================
BUG: KASAN: vmalloc-out-of-bounds in sys_imageblit+0x12f4/0x1430
Write of size 4 at addr ffffc9000b8c1fe0 by task syz-executor.7/29927
CPU: 0 PID: 29927 Comm: syz-executor.7 Tainted: G E
5.13.0-rc5+ #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
Call Trace:
dump_stack+0x141/0x1d7
print_address_description.constprop.0.cold+0x5/0x2f8
kasan_report.cold+0x7c/0xd8
sys_imageblit+0x12f4/0x1430
drm_fbdev_fb_imageblit+0x15c/0x350
soft_cursor+0x514/0xa30
bit_cursor+0xd07/0x1740
fbcon_cursor+0x51d/0x630
hide_cursor+0x85/0x280
putconsxy+0x1d/0x450
vcs_write+0x9cf/0xb90
vfs_write+0x28e/0xa30
ksys_write+0x12d/0x250
do_syscall_64+0x3a/0xb0
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f6c6d34cf59
Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89
f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01
f0 ff ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
RSP: 002b:00007f6c6a832d98 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 000000000069c038 RCX: 00007f6c6d34cf59
RDX: 000000000000008d RSI: 0000000020000040 RDI: 0000000000000003
RBP: 000000000069c038 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 000000000069c044
R13: 00007ffd72091c8f R14: 000000000069c038 R15: 0000000000000001
Memory state around the buggy address:
ffffc9000b8c1e80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
ffffc9000b8c1f00: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
>ffffc9000b8c1f80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
^
ffffc9000b8c2000: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
ffffc9000b8c2080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
==================================================================
in
void sys_imageblit(struct fb_info *p, const struct fb_image *image)
{
u32 fgcolor, bgcolor, start_index, bitstart, pitch_index = 0;
u32 bpl = sizeof(u32), bpp = p->var.bits_per_pixel;
u32 width = image->width;
u32 dx = image->dx, dy = image->dy;
void *dst1;
if (p->state != FBINFO_STATE_RUNNING)
return;
bitstart = (dy * p->fix.line_length * 8) + (dx * bpp);
start_index = bitstart & (32 - 1);
pitch_index = (p->fix.line_length & (bpl - 1)) * 8;
bitstart /= 8;
bitstart &= ~(bpl - 1);
dst1 = (void __force *)p->screen_base + bitstart; // here is the bug
...
}
p->screen_base is alloced by drm_fb_helper_generic_probe and size is
0x3000000
(gdb) bt
#0 drm_fb_helper_generic_probe (fb_helper=0xffff888105b17800,
sizes=<optimized out>) at ../drivers/gpu/drm/drm_fb_helper.c:2342
#1 0xffffffff843f9c6f in drm_fb_helper_single_fb_probe
(preferred_bpp=32, fb_helper=0xffff888105b17800) at
../drivers/gpu/drm/drm_fb_helper.c:1668
#2 __drm_fb_helper_initial_config_and_unlock
(fb_helper=fb_helper@entry=0xffff888105b17800, bpp_sel=bpp_sel@entry=32)
at ../drivers/gpu/drm/drm_fb_helper.c:1826
#3 0xffffffff843fea05 in drm_fb_helper_initial_config
(bpp_sel=<optimized out>, fb_helper=0xffff888105b17800) at
../drivers/gpu/drm/drm_fb_helper.c:1921
#4 drm_fb_helper_initial_config (bpp_sel=<optimized out>,
fb_helper=0xffff888105b17800) at ../drivers/gpu/drm/drm_fb_helper.c:1913
#5 drm_fbdev_client_hotplug (client=client@entry=0xffff888105b17800) at
../drivers/gpu/drm/drm_fb_helper.c:2423
#6 0xffffffff84402504 in drm_fbdev_generic_setup
(dev=dev@entry=0xffff888014632000, preferred_bpp=<optimized out>,
preferred_bpp@entry=32) at ../drivers/gpu/drm/drm_fb_helper.c:2510
#7 0xffffffff84587e71 in bochs_pci_probe (ent=<optimized out>,
pdev=<optimized out>) at ../drivers/gpu/drm/bochs/bochs_drv.c:134
#8 bochs_pci_probe (pdev=<optimized out>, ent=<optimized out>) at
../drivers/gpu/drm/bochs/bochs_drv.c:99
#9 0xffffffff83f7f3d6 in local_pci_probe
(_ddi=_ddi@entry=0xffffc90000647b98) at ../drivers/pci/pci-driver.c:309
#10 0xffffffff83f842e6 in pci_call_probe (id=<optimized out>,
dev=0xffff888015a50000, drv=<optimized out>) at
../drivers/pci/pci-driver.c:366
#11 __pci_device_probe (pci_dev=0xffff888015a50000, drv=<optimized out>)
at ../drivers/pci/pci-driver.c:391
#12 pci_device_probe (dev=0xffff888015a500c8) at
../drivers/pci/pci-driver.c:434
#13 0xffffffff845d5b50 in really_probe
(dev=dev@entry=0xffff888015a500c8, drv=drv@entry=0xffffffff8c21a930
<bochs_pci_driver+112>) at ../drivers/base/dd.c:576
#14 0xffffffff845d686d in driver_probe_device
(drv=drv@entry=0xffffffff8c21a930 <bochs_pci_driver+112>,
dev=dev@entry=0xffff888015a500c8) at ../drivers/base/dd.c:763
#15 0xffffffff845d7397 in device_driver_attach
(drv=drv@entry=0xffffffff8c21a930 <bochs_pci_driver+112>,
dev=dev@entry=0xffff888015a500c8) at ../drivers/base/dd.c:1039
#16 0xffffffff845d7556 in __driver_attach (dev=0xffff888015a500c8,
data=0xffffffff8c21a930 <bochs_pci_driver+112>) at ../drivers/base/dd.c:1117
#17 0xffffffff845cf008 in bus_for_each_dev (bus=<optimized out>,
start=start@entry=0x0 <fixed_percpu_data>,
data=data@entry=0xffffffff8c21a930 <bochs_pci_driver+112>,
fn=fn@entry=0xffffffff845d73d0 <__driver_attach>) at
../drivers/base/bus.c:305
#18 0xffffffff845d477a in driver_attach
(drv=drv@entry=0xffffffff8c21a930 <bochs_pci_driver+112>) at
../drivers/base/dd.c:1133
#19 0xffffffff845d3203 in bus_add_driver
(drv=drv@entry=0xffffffff8c21a930 <bochs_pci_driver+112>) at
../drivers/base/bus.c:622
#20 0xffffffff845d95db in driver_register
(drv=drv@entry=0xffffffff8c21a930 <bochs_pci_driver+112>) at
../drivers/base/driver.c:171
#21 0xffffffff83f7dcba in __pci_register_driver
(drv=drv@entry=0xffffffff8c21a8c0 <bochs_pci_driver>,
owner=owner@entry=0x0 <fixed_percpu_data>,
mod_name=mod_name@entry=0xffffffff89d9c4c0 "bochs_drm") at
../drivers/pci/pci-driver.c:1393
#22 0xffffffff8e85e6cb in bochs_init () at
../drivers/gpu/drm/bochs/bochs_drv.c:191
#23 0xffffffff81002285 in do_one_initcall (fn=0xffffffff8e85e65d
<bochs_init>) at ../init/main.c:1249
#24 0xffffffff8e7ab5f8 in do_initcall_level
(command_line=0xffff888135a06c00 "earlyprintk", level=<optimized out>)
at ../include/linux/compiler.h:234
#25 do_initcalls () at ../init/main.c:1338
#26 do_basic_setup () at ../init/main.c:1358
#27 kernel_init_freeable () at ../init/main.c:1560
#28 0xffffffff8915d45b in kernel_init (unused=<optimized out>) at
../init/main.c:1447
#29 0xffffffff810046ff in ret_from_fork () at
../arch/x86/entry/entry_64.S:294
#30 0x0000000000000000 in ?? ()
(gdb) l
2337
2338 drm_fb_helper_fill_info(fbi, fb_helper, sizes);
2339
2340 if (drm_fbdev_use_shadow_fb(fb_helper)) {
2341 fbi->screen_buffer = vzalloc(fbi->screen_size);
2342 if (!fbi->screen_buffer)
2343 return -ENOMEM;
2344
2345 fbi->fbdefio = &drm_fbdev_defio;
2346
(gdb) p/x fbi->screen_size
$1 = 0x300000
(gdb) p/x fbi->screen_buffer
$2 = 0xffffc9000c561000
(gdb) p/x fbi->screen_base
$3 = 0xffffc9000c561000
when run the poc :
Thread 1 hit Breakpoint 2, updatescrollmode
(info=info@entry=0xffff8881081bd000, vc=vc@entry=0xffff888010479000,
p=<optimized out>) at ../drivers/video/fbdev/core/fbcon.c:1950
1950 static void updatescrollmode(struct fbcon_display *p,
(gdb) frame 1
#1 0xffffffff8401d9c3 in fbcon_resize (vc=vc@entry=0xffff888010479000,
width=width@entry=20, height=height@entry=7, user=user@entry=0) at
../drivers/video/fbdev/core/fbcon.c:2030
2030 updatescrollmode(p, info, vc);
(gdb) p p->vrows
$5 = 48
(gdb) p &p->vrows
$6 = (int *) 0xffffffff90056094 <fb_display+20>
(gdb) frame 0
#0 updatescrollmode (info=info@entry=0xffff8881081bd000,
vc=vc@entry=0xffff888010479000, p=<optimized out>) at
../drivers/video/fbdev/core/fbcon.c:1950
1950 static void updatescrollmode(struct fbcon_display *p,
(gdb) n
1954 struct fbcon_ops *ops = info->fbcon_par;
(gdb)
1955 int fh = vc->vc_font.height;
(gdb)
1956 int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
(gdb)
1957 int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual,
(gdb)
1960 p->vrows = vyres/fh;
(gdb)
1961 if (yres > (fh * (vc->vc_rows + 1)))
(gdb) p info.var.yres_virtual
$8 = 0
(gdb) p info.var.xres_virtual
$9 = 0
(gdb) p info.var.xres
$10 = 160
(gdb) p info.var.yres
$11 = 120
(gdb) p yres
$12 = 120
(gdb) n
1963 if ((yres % fh) && (vyres % fh < yres % fh))
(gdb) n
1964 p->vrows--;
(gdb) x/1wd 0xffffffff90056094
0xffffffff90056094 <fb_display+20>: -1
(gdb) p vc->vc_font
$19 = {width = 8, height = 16, charcount = 256, data =
0xffffffff89c532d0 <fontdata_8x16+16> ""}
the kasan backtrace debug in gdb
Thread 2 hit Breakpoint 6, sys_imageblit (p=p@entry=0xffff8881081bd000,
image=image@entry=0xffff888011f85080) at
../drivers/video/fbdev/core/sysimgblt.c:244
244 u32 width = image->width;
(gdb) n
248 if (p->state != FBINFO_STATE_RUNNING)
(gdb)
251 bitstart = (dy * p->fix.line_length * 8) + (dx * bpp);
(gdb)
252 start_index = bitstart & (32 - 1);
(gdb)
253 pitch_index = (p->fix.line_length & (bpl - 1)) * 8;
(gdb)
257 dst1 = (void __force *)p->screen_base + bitstart;
(gdb) n
259 if (p->fbops->fb_sync)
(gdb) p dst1
$23 = (void *) 0xffffc9000c5a1220
(gdb) p p->screen_base
$24 = 0xffffc9000c561000 ""
(gdb) p/x 0xffffc9000c5a1220-0xffffc9000c561000
$25 = 0x40220
(gdb) bt
#0 sys_imageblit (p=p@entry=0xffff8881081bd000,
image=image@entry=0xffff888011f85080) at
../drivers/video/fbdev/core/sysimgblt.c:259
#1 0xffffffff843fbbcc in drm_fb_helper_sys_imageblit
(info=info@entry=0xffff8881081bd000,
image=image@entry=0xffff888011f85080) at
../drivers/gpu/drm/drm_fb_helper.c:794
#2 0xffffffff843fc831 in drm_fbdev_fb_imageblit
(info=0xffff8881081bd000, image=0xffff888011f85080) at
../drivers/gpu/drm/drm_fb_helper.c:2276
#3 0xffffffff8402eff4 in soft_cursor
(info=info@entry=0xffff8881081bd000,
cursor=cursor@entry=0xffffc9001090f9f0) at
../drivers/video/fbdev/core/softcursor.c:74
#4 0xffffffff8402deb2 in bit_cursor (vc=vc@entry=0xffff888010479000,
info=info@entry=0xffff8881081bd000, mode=mode@entry=1, fg=<optimized
out>, bg=bg@entry=0) at ../drivers/video/fbdev/core/bitblit.c:377
#5 0xffffffff84019d5e in fbcon_cursor (vc=0xffff888010479000, mode=1)
at ../drivers/video/fbdev/core/fbcon.c:1339
#6 0xffffffff842a5155 in set_cursor (vc=0xffff888010479000) at
../drivers/tty/vt/vt.c:920
#7 set_cursor (vc=0xffff888010479000) at ../drivers/tty/vt/vt.c:911
#8 0xffffffff842a6135 in redraw_screen (vc=vc@entry=0xffff888010479000,
is_switch=is_switch@entry=0) at ../drivers/tty/vt/vt.c:1037
#9 0xffffffff8401c46c in fbcon_modechanged
(info=info@entry=0xffff8881081bd000) at
../drivers/video/fbdev/core/fbcon.c:2651
#10 0xffffffff8401d2da in fbcon_update_vcs
(info=info@entry=0xffff8881081bd000, all=all@entry=false) at
../drivers/video/fbdev/core/fbcon.c:2696
#11 0xffffffff83ffa426 in do_fb_ioctl
(info=info@entry=0xffff8881081bd000, cmd=cmd@entry=17921,
arg=arg@entry=536870912) at ../drivers/video/fbdev/core/fbmem.c:1110
#12 0xffffffff83ffa577 in fb_ioctl (file=file@entry=0xffff88810e274500,
cmd=cmd@entry=17921, arg=arg@entry=536870912) at
../drivers/video/fbdev/core/fbmem.c:1185
#13 0xffffffff81cab763 in vfs_ioctl (arg=536870912, cmd=17921,
filp=0xffff88810e274500) at ../fs/ioctl.c:51
#14 __do_sys_ioctl (arg=536870912, cmd=17921, fd=3) at ../fs/ioctl.c:1069
#15 __se_sys_ioctl (arg=536870912, cmd=17921, fd=3) at ../fs/ioctl.c:1055
#16 __x64_sys_ioctl (regs=<optimized out>) at ../fs/ioctl.c:1055
#17 0xffffffff89158596 in do_syscall_64 (nr=<optimized out>,
regs=0xffffc9001090ff58) at ../arch/x86/entry/common.c:47
#18 0xffffffff89200068 in entry_SYSCALL_64 () at
../arch/x86/entry/entry_64.S:112
#19 0x0000000000000000 in ?? ()
(gdb) frame 0
#0 sys_imageblit (p=p@entry=0xffff8881081bd000,
image=image@entry=0xffff888011f85080) at
../drivers/video/fbdev/core/sysimgblt.c:259
259 if (p->fbops->fb_sync)
(gdb) p image.dy
$29 = 64
(gdb) p image.dx
$30 = 136
(gdb) frame 1
#1 0xffffffff843fbbcc in drm_fb_helper_sys_imageblit
(info=info@entry=0xffff8881081bd000,
image=image@entry=0xffff888011f85080) at
../drivers/gpu/drm/drm_fb_helper.c:794
794 sys_imageblit(info, image);
(gdb) p info.screen_
screen_base screen_buffer screen_size
(gdb) p info.screen_size
$31 = 3145728
(gdb) p/x info.screen_size
$32 = 0x300000
(gdb) frame 4
#4 0xffffffff8402deb2 in bit_cursor (vc=vc@entry=0xffff888010479000,
info=info@entry=0xffff8881081bd000, mode=mode@entry=1, fg=<optimized
out>, bg=bg@entry=0) at ../drivers/video/fbdev/core/bitblit.c:377
377 soft_cursor(info, &cursor);
(gdb) p ops->p.vrows
$38 = -1
(gdb) p &ops->p.vrows
$39 = (int *) 0xffffffff90056094 <fb_display+20>
在 2021/7/27 23:10, Tetsuo Handa 写道:
> OK, test.c was generated by syzkaller, and I guess that
> your team is running syzkaller in your environment.
>
> It seems that test.c is doing
>
> struct fb_var_screeninfo var = { ... };
> int fd_fb0 = open("/dev/fb0", O_RDONLY);
> ioctl(fd_fb0, FBIOPUT_VSCREENINFO, &var);
>
> int fd_vcs = open("/dev/vcsa5", O_WRONLY|O_NOCTTY|O_NONBLOCK);
> write(fd_vcs, "\16\272P\323\236\3\377\231\34\0\0\0", 12);
>
> in parallel and hitting some race.
>
> I don't have environment to test with KASAN now. But syzbot dashboard
> indicates that crash in sys_imageblit is not happening recently.
>
> KASAN: vmalloc-out-of-bounds Write in sys_imageblit
> https://syzkaller.appspot.com/bug?id=d2e785fe8b66425f3a2be0a209b08456f8200b53
>
> BUG: unable to handle kernel paging request in sys_imageblit
> https://syzkaller.appspot.com/bug?id=26d6ef43f58d17c6899c63a53544ee9bb26d0bfd
>
> You need to provide more information.
> Are you seeing this problem with v5.14-rc3 ? Please provide full oops report.
> Please show a URL for this problem if this problem was reported somewhere.
>
Hi Tetsuo,
On Mon, Aug 30, 2021 at 12:25 PM Tetsuo Handa
<[email protected]> wrote:
> On 2021/08/30 17:16, Daniel Vetter wrote:
> > On Mon, Aug 30, 2021 at 11:30:23AM +0800, [email protected] wrote:
> >> From: Haimin Zhang <[email protected]>
> >>
> >> 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 <[email protected]>
> >> Signed-off-by: Tetsuo Handa <[email protected]>
>
> 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: <[email protected]>
> 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
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds