* [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2)
@ 2021-11-19 9:18 syzbot
2022-01-20 22:58 ` syzbot
` (4 more replies)
0 siblings, 5 replies; 28+ messages in thread
From: syzbot @ 2021-11-19 9:18 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: fa55b7dcdc43 Linux 5.16-rc1
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15fe2569b00000
kernel config: https://syzkaller.appspot.com/x/.config?x=6d3b8fd1977c1e73
dashboard link: https://syzkaller.appspot.com/bug?extid=14b0e8f3fd1612e35350
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
userspace arch: i386
Unfortunately, I don't have any reproducer for this issue yet.
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com
524155 pages RAM
0 pages HighMem/MovableOnly
163742 pages reserved
0 pages cma reserved
==================================================================
BUG: KASAN: vmalloc-out-of-bounds in fast_imageblit drivers/video/fbdev/core/sysimgblt.c:229 [inline]
BUG: KASAN: vmalloc-out-of-bounds in sys_imageblit+0x12f4/0x1430 drivers/video/fbdev/core/sysimgblt.c:275
Write of size 4 at addr ffffc90004631000 by task syz-executor.0/7913
CPU: 0 PID: 7913 Comm: syz-executor.0 Not tainted 5.16.0-rc1-syzkaller #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
print_address_description.constprop.0.cold+0xf/0x320 mm/kasan/report.c:247
__kasan_report mm/kasan/report.c:433 [inline]
kasan_report.cold+0x83/0xdf mm/kasan/report.c:450
fast_imageblit drivers/video/fbdev/core/sysimgblt.c:229 [inline]
sys_imageblit+0x12f4/0x1430 drivers/video/fbdev/core/sysimgblt.c:275
drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:794 [inline]
drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2282
bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:124 [inline]
bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:173
fbcon_putcs+0x353/0x440 drivers/video/fbdev/core/fbcon.c:1277
do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676
redraw_screen+0x61f/0x740 drivers/tty/vt/vt.c:1035
fbcon_modechanged+0x58c/0x6c0 drivers/video/fbdev/core/fbcon.c:2182
fbcon_update_vcs+0x3a/0x50 drivers/video/fbdev/core/fbcon.c:2227
do_fb_ioctl+0x62e/0x690 drivers/video/fbdev/core/fbmem.c:1114
fb_compat_ioctl+0x17e/0x610 drivers/video/fbdev/core/fbmem.c:1313
__do_compat_sys_ioctl+0x1c7/0x290 fs/ioctl.c:972
do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline]
__do_fast_syscall_32+0x65/0xf0 arch/x86/entry/common.c:178
do_fast_syscall_32+0x2f/0x70 arch/x86/entry/common.c:203
entry_SYSENTER_compat_after_hwframe+0x4d/0x5c
RIP: 0023:0xf6e67549
Code: 03 74 c0 01 10 05 03 74 b8 01 10 06 03 74 b4 01 10 07 03 74 b0 01 10 08 03 74 d8 01 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 8d b4 26 00 00 00 00 8d b4 26 00 00 00 00
RSP: 002b:00000000f44615fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 0000000000004601
RDX: 0000000020000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
</TASK>
Memory state around the buggy address:
ffffc90004630f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffffc90004630f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffffc90004631000: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
^
ffffc90004631080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
ffffc90004631100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
==================================================================
----------------
Code disassembly (best guess):
0: 03 74 c0 01 add 0x1(%rax,%rax,8),%esi
4: 10 05 03 74 b8 01 adc %al,0x1b87403(%rip) # 0x1b8740d
a: 10 06 adc %al,(%rsi)
c: 03 74 b4 01 add 0x1(%rsp,%rsi,4),%esi
10: 10 07 adc %al,(%rdi)
12: 03 74 b0 01 add 0x1(%rax,%rsi,4),%esi
16: 10 08 adc %cl,(%rax)
18: 03 74 d8 01 add 0x1(%rax,%rbx,8),%esi
1c: 00 00 add %al,(%rax)
1e: 00 00 add %al,(%rax)
20: 00 51 52 add %dl,0x52(%rcx)
23: 55 push %rbp
24: 89 e5 mov %esp,%ebp
26: 0f 34 sysenter
28: cd 80 int $0x80
* 2a: 5d pop %rbp <-- trapping instruction
2b: 5a pop %rdx
2c: 59 pop %rcx
2d: c3 retq
2e: 90 nop
2f: 90 nop
30: 90 nop
31: 90 nop
32: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi
39: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2021-11-19 9:18 [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) syzbot @ 2022-01-20 22:58 ` syzbot 2022-01-21 1:48 ` syzbot ` (3 subsequent siblings) 4 siblings, 0 replies; 28+ messages in thread From: syzbot @ 2022-01-20 22:58 UTC (permalink / raw) To: deller, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs syzbot has found a reproducer for the following issue on: HEAD commit: 7fc5253f5a13 Add linux-next specific files for 20220120 git tree: linux-next console output: https://syzkaller.appspot.com/x/log.txt?x=16385270700000 kernel config: https://syzkaller.appspot.com/x/.config?x=94e8da4df9ab6319 dashboard link: https://syzkaller.appspot.com/bug?extid=14b0e8f3fd1612e35350 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=155dde3db00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=125298e0700000 IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com BUG: unable to handle page fault for address: fffff520008b2208 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 23ffed067 P4D 23ffed067 PUD 10db4067 PMD 1470c4067 PTE 0 Oops: 0000 [#1] PREEMPT SMP KASAN CPU: 0 PID: 3595 Comm: syz-executor362 Not tainted 5.16.0-next-20220120-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:fast_imageblit drivers/video/fbdev/core/sysimgblt.c:229 [inline] RIP: 0010:sys_imageblit+0x656/0x1430 drivers/video/fbdev/core/sysimgblt.c:275 Code: 14 38 48 89 d8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 b6 0c 00 00 8b 44 24 20 23 03 8b 5c 24 18 31 c3 48 89 e8 48 c1 e8 03 <42> 0f b6 14 38 48 89 e8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 RSP: 0018:ffffc90002a1f368 EFLAGS: 00010a02 RAX: 1ffff920008b2208 RBX: 0000000000000000 RCX: 0000000000000007 RDX: 0000000000000000 RSI: ffffffff84257bf0 RDI: 0000000000000003 RBP: ffffc90004591040 R08: 000000000000001f R09: ffffffff84257a74 R10: ffffffff84257be1 R11: 0000000000000020 R12: 0000000000000007 R13: 00000000000003ef R14: ffff888146efc7e0 R15: dffffc0000000000 FS: 0000555555c5d300(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: fffff520008b2208 CR3: 0000000023b12000 CR4: 00000000003506f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:794 [inline] drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2288 bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:124 [inline] bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:173 fbcon_putcs+0x353/0x440 drivers/video/fbdev/core/fbcon.c:1277 do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 invert_screen+0x1d4/0x600 drivers/tty/vt/vt.c:800 highlight drivers/tty/vt/selection.c:57 [inline] clear_selection drivers/tty/vt/selection.c:84 [inline] clear_selection+0x55/0x70 drivers/tty/vt/selection.c:80 vc_do_resize+0xe6e/0x1180 drivers/tty/vt/vt.c:1257 fbcon_do_set_font+0x47a/0x760 drivers/video/fbdev/core/fbcon.c:1928 fbcon_set_font+0x817/0xa00 drivers/video/fbdev/core/fbcon.c:2014 con_font_set drivers/tty/vt/vt.c:4666 [inline] con_font_op+0x73a/0xc90 drivers/tty/vt/vt.c:4710 vt_k_ioctl drivers/tty/vt/vt_ioctl.c:474 [inline] vt_ioctl+0x1e26/0x2b10 drivers/tty/vt/vt_ioctl.c:752 tty_ioctl+0xbbd/0x1660 drivers/tty/tty_io.c:2778 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:874 [inline] __se_sys_ioctl fs/ioctl.c:860 [inline] __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:860 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae RIP: 0033:0x7f3bac0e1349 Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 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 c7 c1 c0 ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007ffff160a718 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f3bac0e1349 RDX: 0000000020000000 RSI: 0000000000004b72 RDI: 0000000000000004 RBP: 00007f3bac0a51d0 R08: 000000000000000d R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3bac0a5260 R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 </TASK> Modules linked in: CR2: fffff520008b2208 ---[ end trace 0000000000000000 ]--- RIP: 0010:fast_imageblit drivers/video/fbdev/core/sysimgblt.c:229 [inline] RIP: 0010:sys_imageblit+0x656/0x1430 drivers/video/fbdev/core/sysimgblt.c:275 Code: 14 38 48 89 d8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 b6 0c 00 00 8b 44 24 20 23 03 8b 5c 24 18 31 c3 48 89 e8 48 c1 e8 03 <42> 0f b6 14 38 48 89 e8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 RSP: 0018:ffffc90002a1f368 EFLAGS: 00010a02 RAX: 1ffff920008b2208 RBX: 0000000000000000 RCX: 0000000000000007 RDX: 0000000000000000 RSI: ffffffff84257bf0 RDI: 0000000000000003 RBP: ffffc90004591040 R08: 000000000000001f R09: ffffffff84257a74 R10: ffffffff84257be1 R11: 0000000000000020 R12: 0000000000000007 R13: 00000000000003ef R14: ffff888146efc7e0 R15: dffffc0000000000 FS: 0000555555c5d300(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: fffff520008b2208 CR3: 0000000023b12000 CR4: 00000000003506f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 ---------------- Code disassembly (best guess): 0: 14 38 adc $0x38,%al 2: 48 89 d8 mov %rbx,%rax 5: 83 e0 07 and $0x7,%eax 8: 83 c0 03 add $0x3,%eax b: 38 d0 cmp %dl,%al d: 7c 08 jl 0x17 f: 84 d2 test %dl,%dl 11: 0f 85 b6 0c 00 00 jne 0xccd 17: 8b 44 24 20 mov 0x20(%rsp),%eax 1b: 23 03 and (%rbx),%eax 1d: 8b 5c 24 18 mov 0x18(%rsp),%ebx 21: 31 c3 xor %eax,%ebx 23: 48 89 e8 mov %rbp,%rax 26: 48 c1 e8 03 shr $0x3,%rax * 2a: 42 0f b6 14 38 movzbl (%rax,%r15,1),%edx <-- trapping instruction 2f: 48 89 e8 mov %rbp,%rax 32: 83 e0 07 and $0x7,%eax 35: 83 c0 03 add $0x3,%eax 38: 38 d0 cmp %dl,%al 3a: 7c 08 jl 0x44 3c: 84 d2 test %dl,%dl 3e: 0f .byte 0xf 3f: 85 .byte 0x85 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2021-11-19 9:18 [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) syzbot 2022-01-20 22:58 ` syzbot @ 2022-01-21 1:48 ` syzbot 2022-07-29 6:51 ` Khalid Masum ` (2 subsequent siblings) 4 siblings, 0 replies; 28+ messages in thread From: syzbot @ 2022-01-21 1:48 UTC (permalink / raw) To: bugs-a21, deller, dri-devel, javierm, linux-fbdev, linux-kernel, maxime, syzkaller-bugs syzbot has bisected this issue to: commit 0499f419b76f94ede08304aad5851144813ac55c Author: Javier Martinez Canillas <javierm@redhat.com> Date: Mon Jan 10 09:56:25 2022 +0000 video: vga16fb: Only probe for EGA and VGA 16 color graphic cards bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=14c71e37b00000 start commit: 7fc5253f5a13 Add linux-next specific files for 20220120 git tree: linux-next final oops: https://syzkaller.appspot.com/x/report.txt?x=16c71e37b00000 console output: https://syzkaller.appspot.com/x/log.txt?x=12c71e37b00000 kernel config: https://syzkaller.appspot.com/x/.config?x=94e8da4df9ab6319 dashboard link: https://syzkaller.appspot.com/bug?extid=14b0e8f3fd1612e35350 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=155dde3db00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=125298e0700000 Reported-by: syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com Fixes: 0499f419b76f ("video: vga16fb: Only probe for EGA and VGA 16 color graphic cards") For information about bisection process see: https://goo.gl/tpsmEJ#bisection ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2021-11-19 9:18 [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) syzbot 2022-01-20 22:58 ` syzbot 2022-01-21 1:48 ` syzbot @ 2022-07-29 6:51 ` Khalid Masum 2022-07-30 17:25 ` Helge Deller 2022-07-30 8:12 ` Khalid Masum 2022-07-30 11:45 ` Khalid Masum 4 siblings, 1 reply; 28+ messages in thread From: Khalid Masum @ 2022-07-29 6:51 UTC (permalink / raw) To: syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs Here is a simplified reproducer for the issue: https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-07-29 6:51 ` Khalid Masum @ 2022-07-30 17:25 ` Helge Deller 2022-07-30 18:49 ` [PATCH] tty: vt: selection: Add check for valid tiocl_selection values Helge Deller ` (3 more replies) 0 siblings, 4 replies; 28+ messages in thread From: Helge Deller @ 2022-07-30 17:25 UTC (permalink / raw) To: Khalid Masum, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby On 7/29/22 08:51, Khalid Masum wrote: > Here is a simplified reproducer for the issue: > > https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c The reproducer does this: ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3 ys:0 xe:0 ye:0 mode:0) = 0 -> sets the text selection area ioctl(4, KDFONTOP) with op=0 (con_font_set), charcount=512 width=8 height=32, 0x20000000) = 0 -> changes the font size. It does not crash with current Linus' head (v5.19-rc8). Kernel v5.16, which was used by this KASAN report, hasn't received backports since months, so I tried stable kernel v5.15.58 instead, and this kernel crashed with the reproducer. The reproducer brings up two issues with current code: 1. The reproducer uses ioctl(TIOCLINUX, TIOCL_SETSEL) and hands over (invalid) zero-values for ys and ye for the starting lines. This is wrong, since the API seems to expect a "1" as the very first line for the selection. This can be easily fixed by adding checks for zero-values and return -EINVAL if found. But this bug isn't critical itself and is not the reason for the kernel crash. Without the checks, the ioctl handler simply wraps the coordinate values and converts them from: input selection: xs:3 ys:0 xe:0 ye:0 mode:0 to the new: vc_selection = xs:2 ys:23 xe:127 ye:23 mode:0 which is the current maximum coordinates for the screen. Those higher values now trigger issue #2: After the TIOCL_SETSEL the last line on the screen is now selected. The KDFONTOP ioctl then sets a 8x32 console font, and replaces the former 8x16 console font. With the bigger font the current screen selection is now outside the visible screen and this finally triggeres this backtrace, because vc_do_resize() calls clear_selection() to unhighlight the selection (which starts to render chars outside of the screen): drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:794 [inline] drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2288 bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:124 [inline] bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:173 fbcon_putcs+0x353/0x440 drivers/video/fbdev/core/fbcon.c:1277 do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 invert_screen+0x1d4/0x600 drivers/tty/vt/vt.c:800 highlight drivers/tty/vt/selection.c:57 [inline] clear_selection drivers/tty/vt/selection.c:84 [inline] clear_selection+0x55/0x70 drivers/tty/vt/selection.c:80 vc_do_resize+0xe6e/0x1180 drivers/tty/vt/vt.c:1257 IMHO the easiest way to prevent this crash is to simply clear the selection before the various con_font_set() console handlers are called. Otherwise every console driver needs to add checks and verify if the current selection still fits with the selected font, which gets tricky because some of those drivers fiddle with the screen width&height before calling vc_do_resize(). I'll follow up to this mail with patches for both issues shortly. Helge ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] tty: vt: selection: Add check for valid tiocl_selection values 2022-07-30 17:25 ` Helge Deller @ 2022-07-30 18:49 ` Helge Deller 2022-08-04 5:47 ` Jiri Slaby 2022-07-30 18:50 ` [PATCH] vt: Clear selection before changing the font Helge Deller ` (2 subsequent siblings) 3 siblings, 1 reply; 28+ messages in thread From: Helge Deller @ 2022-07-30 18:49 UTC (permalink / raw) To: Khalid Masum, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby The line and column numbers for the selection need to start at 1. Add the checks to prevent invalid input. Signed-off-by: Helge Deller <deller@gmx.de> Reported-by: syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index f7755e73696e..58692a9b4097 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -326,6 +326,9 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, return 0; } + if (!v->xs || !v->ys || !v->xe || !v->ye) + return -EINVAL; + v->xs = min_t(u16, v->xs - 1, vc->vc_cols - 1); v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] tty: vt: selection: Add check for valid tiocl_selection values 2022-07-30 18:49 ` [PATCH] tty: vt: selection: Add check for valid tiocl_selection values Helge Deller @ 2022-08-04 5:47 ` Jiri Slaby 2022-08-04 7:15 ` Helge Deller 0 siblings, 1 reply; 28+ messages in thread From: Jiri Slaby @ 2022-08-04 5:47 UTC (permalink / raw) To: Helge Deller, Khalid Masum, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman On 30. 07. 22, 20:49, Helge Deller wrote: > The line and column numbers for the selection need to start at 1. > Add the checks to prevent invalid input. > > Signed-off-by: Helge Deller <deller@gmx.de> > Reported-by: syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com > > diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c > index f7755e73696e..58692a9b4097 100644 > --- a/drivers/tty/vt/selection.c > +++ b/drivers/tty/vt/selection.c > @@ -326,6 +326,9 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, > return 0; > } > > + if (!v->xs || !v->ys || !v->xe || !v->ye) > + return -EINVAL; Hmm, I'm not sure about this. It potentially breaks userspace (by returning EINVAL now). And the code below should handle this just fine, right: > + > v->xs = min_t(u16, v->xs - 1, vc->vc_cols - 1); > v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); > v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); ? thanks, -- js suse labs ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] tty: vt: selection: Add check for valid tiocl_selection values 2022-08-04 5:47 ` Jiri Slaby @ 2022-08-04 7:15 ` Helge Deller 2022-08-04 8:44 ` Helge Deller 0 siblings, 1 reply; 28+ messages in thread From: Helge Deller @ 2022-08-04 7:15 UTC (permalink / raw) To: Jiri Slaby, Khalid Masum, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman Hello Jiri, Thanks for looking into this patch! On 8/4/22 07:47, Jiri Slaby wrote: > On 30. 07. 22, 20:49, Helge Deller wrote: >> The line and column numbers for the selection need to start at 1. >> Add the checks to prevent invalid input. >> >> Signed-off-by: Helge Deller <deller@gmx.de> >> Reported-by: syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com >> >> diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c >> index f7755e73696e..58692a9b4097 100644 >> --- a/drivers/tty/vt/selection.c >> +++ b/drivers/tty/vt/selection.c >> @@ -326,6 +326,9 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, >> return 0; >> } >> >> + if (!v->xs || !v->ys || !v->xe || !v->ye) >> + return -EINVAL; > > Hmm, I'm not sure about this. It potentially breaks userspace (by > returning EINVAL now). Right. According to the code below, my interpretation is that all xs/ys/xe/ye values should be > 0. But of course I might be wrong on this, as I didn't find any documentation for TIOCL_SETSEL. And if userspace tries to set an invalid selection (e.g. by selecting row 0), my patch now returns -EINVAL, while it returned success before. > And the code below should handle this just fine, right: >> v->xs = min_t(u16, v->xs - 1, vc->vc_cols - 1); >> v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); >> v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); It "handles it fine" in the sense that it can cope with the input and will not crash. But it returns (maybe?) unexpected results... For example, if a user selects row 0 (where I assume he wanted to set the first line), he instead selects the last row. I'm not sure if this is the expected behaviour. Do you know of any userspace program which breaks because of this? Helge ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] tty: vt: selection: Add check for valid tiocl_selection values 2022-08-04 7:15 ` Helge Deller @ 2022-08-04 8:44 ` Helge Deller 2022-08-04 9:22 ` Jiri Slaby 0 siblings, 1 reply; 28+ messages in thread From: Helge Deller @ 2022-08-04 8:44 UTC (permalink / raw) To: Jiri Slaby, Khalid Masum, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman On 8/4/22 09:15, Helge Deller wrote: > Hello Jiri, > > Thanks for looking into this patch! > > On 8/4/22 07:47, Jiri Slaby wrote: >> On 30. 07. 22, 20:49, Helge Deller wrote: >>> The line and column numbers for the selection need to start at 1. >>> Add the checks to prevent invalid input. >>> >>> Signed-off-by: Helge Deller <deller@gmx.de> >>> Reported-by: syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com >>> >>> diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c >>> index f7755e73696e..58692a9b4097 100644 >>> --- a/drivers/tty/vt/selection.c >>> +++ b/drivers/tty/vt/selection.c >>> @@ -326,6 +326,9 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, >>> return 0; >>> } >>> >>> + if (!v->xs || !v->ys || !v->xe || !v->ye) >>> + return -EINVAL; >> >> Hmm, I'm not sure about this. It potentially breaks userspace (by >> returning EINVAL now). > > Right. > According to the code below, my interpretation is that all xs/ys/xe/ye values > should be > 0. But of course I might be wrong on this, as I didn't find any > documentation for TIOCL_SETSEL. > > And if userspace tries to set an invalid selection (e.g. by selecting row 0), > my patch now returns -EINVAL, while it returned success before. > >> And the code below should handle this just fine, right: >>> v->xs = min_t(u16, v->xs - 1, vc->vc_cols - 1); >>> v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); >>> v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); > > It "handles it fine" in the sense that it can cope with the > input and will not crash. > But it returns (maybe?) unexpected results... After some more thinking maybe you are right. In case a user provided invalid values in the past, simply an unexpected selection was set, but nothing broke. Since the patch doesn't fix any critical issue, we could just drop this patch and leave it as is. Helge ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] tty: vt: selection: Add check for valid tiocl_selection values 2022-08-04 8:44 ` Helge Deller @ 2022-08-04 9:22 ` Jiri Slaby 2022-08-05 11:13 ` Adam Borowski 0 siblings, 1 reply; 28+ messages in thread From: Jiri Slaby @ 2022-08-04 9:22 UTC (permalink / raw) To: Helge Deller, Khalid Masum, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman On 04. 08. 22, 10:44, Helge Deller wrote: > On 8/4/22 09:15, Helge Deller wrote: >> Hello Jiri, >> >> Thanks for looking into this patch! >> >> On 8/4/22 07:47, Jiri Slaby wrote: >>> On 30. 07. 22, 20:49, Helge Deller wrote: >>>> The line and column numbers for the selection need to start at 1. >>>> Add the checks to prevent invalid input. >>>> >>>> Signed-off-by: Helge Deller <deller@gmx.de> >>>> Reported-by: syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com >>>> >>>> diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c >>>> index f7755e73696e..58692a9b4097 100644 >>>> --- a/drivers/tty/vt/selection.c >>>> +++ b/drivers/tty/vt/selection.c >>>> @@ -326,6 +326,9 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, >>>> return 0; >>>> } >>>> >>>> + if (!v->xs || !v->ys || !v->xe || !v->ye) >>>> + return -EINVAL; >>> >>> Hmm, I'm not sure about this. It potentially breaks userspace (by >>> returning EINVAL now). >> >> Right. >> According to the code below, my interpretation is that all xs/ys/xe/ye values >> should be > 0. But of course I might be wrong on this, as I didn't find any >> documentation for TIOCL_SETSEL. >> >> And if userspace tries to set an invalid selection (e.g. by selecting row 0), >> my patch now returns -EINVAL, while it returned success before. >> >>> And the code below should handle this just fine, right: >>>> v->xs = min_t(u16, v->xs - 1, vc->vc_cols - 1); >>>> v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); >>>> v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); >> >> It "handles it fine" in the sense that it can cope with the >> input and will not crash. >> But it returns (maybe?) unexpected results... > > After some more thinking maybe you are right. > In case a user provided invalid values in the past, simply an unexpected > selection was set, but nothing broke. > Since the patch doesn't fix any critical issue, we could just drop this patch > and leave it as is. We can still do a trial and revert it if something breaks... It's just that _noone_ knows with all this undocumented stuff ;). But in fact, 0 currently means full row/column. Isn't it on purpose? Today, we are out of luck, codesearch.debian.net gives no clue about users: https://codesearch.debian.net/search?q=%5CbTIOCL_SETSEL%5Cb&literal=0 thanks, -- js suse labs ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] tty: vt: selection: Add check for valid tiocl_selection values 2022-08-04 9:22 ` Jiri Slaby @ 2022-08-05 11:13 ` Adam Borowski 0 siblings, 0 replies; 28+ messages in thread From: Adam Borowski @ 2022-08-05 11:13 UTC (permalink / raw) To: Jiri Slaby Cc: Helge Deller, Khalid Masum, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman On Thu, Aug 04, 2022 at 11:22:26AM +0200, Jiri Slaby wrote: > On 04. 08. 22, 10:44, Helge Deller wrote: > > On 8/4/22 09:15, Helge Deller wrote: > > > On 8/4/22 07:47, Jiri Slaby wrote: > > > > On 30. 07. 22, 20:49, Helge Deller wrote: > > > > > The line and column numbers for the selection need to start at 1. > > > > > Add the checks to prevent invalid input. > > > > > --- a/drivers/tty/vt/selection.c > > > > > +++ b/drivers/tty/vt/selection.c > > > > > @@ -326,6 +326,9 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, > > > > > + if (!v->xs || !v->ys || !v->xe || !v->ye) > > > > > + return -EINVAL; > > > > > > > > Hmm, I'm not sure about this. It potentially breaks userspace (by > > > > returning EINVAL now). > We can still do a trial and revert it if something breaks... It's just that > _noone_ knows with all this undocumented stuff ;). > > But in fact, 0 currently means full row/column. Isn't it on purpose? > > Today, we are out of luck, codesearch.debian.net gives no clue about users: > https://codesearch.debian.net/search?q=%5CbTIOCL_SETSEL%5Cb&literal=0 That's because the macro is undocumented. "man ioctl_console" says: TIOCLINUX, subcode=2 Set selection. argp points to a [...] thus everyone writes it as a number. You'd need to grep for TIOCLINUX; there's not that many references to check... Meow! -- ⢀⣴⠾⠻⢶⣦⠀ ⣾⠁⢠⠒⠀⣿⡁ Say what you want about Adolf, at least he was the man who ⢿⡄⠘⠷⠚⠋⠀ killed Hitler. Your turn, Vlad! ⠈⠳⣄⠀⠀⠀⠀ ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] tty: vt: selection: Add check for valid tiocl_selection values @ 2022-08-05 11:13 ` Adam Borowski 0 siblings, 0 replies; 28+ messages in thread From: Adam Borowski @ 2022-08-05 11:13 UTC (permalink / raw) To: Jiri Slaby Cc: linux-fbdev, Khalid Masum, Greg Kroah-Hartman, Helge Deller, syzkaller-bugs, linux-kernel, dri-devel, syzbot On Thu, Aug 04, 2022 at 11:22:26AM +0200, Jiri Slaby wrote: > On 04. 08. 22, 10:44, Helge Deller wrote: > > On 8/4/22 09:15, Helge Deller wrote: > > > On 8/4/22 07:47, Jiri Slaby wrote: > > > > On 30. 07. 22, 20:49, Helge Deller wrote: > > > > > The line and column numbers for the selection need to start at 1. > > > > > Add the checks to prevent invalid input. > > > > > --- a/drivers/tty/vt/selection.c > > > > > +++ b/drivers/tty/vt/selection.c > > > > > @@ -326,6 +326,9 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, > > > > > + if (!v->xs || !v->ys || !v->xe || !v->ye) > > > > > + return -EINVAL; > > > > > > > > Hmm, I'm not sure about this. It potentially breaks userspace (by > > > > returning EINVAL now). > We can still do a trial and revert it if something breaks... It's just that > _noone_ knows with all this undocumented stuff ;). > > But in fact, 0 currently means full row/column. Isn't it on purpose? > > Today, we are out of luck, codesearch.debian.net gives no clue about users: > https://codesearch.debian.net/search?q=%5CbTIOCL_SETSEL%5Cb&literal=0 That's because the macro is undocumented. "man ioctl_console" says: TIOCLINUX, subcode=2 Set selection. argp points to a [...] thus everyone writes it as a number. You'd need to grep for TIOCLINUX; there's not that many references to check... Meow! -- ⢀⣴⠾⠻⢶⣦⠀ ⣾⠁⢠⠒⠀⣿⡁ Say what you want about Adolf, at least he was the man who ⢿⡄⠘⠷⠚⠋⠀ killed Hitler. Your turn, Vlad! ⠈⠳⣄⠀⠀⠀⠀ ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] vt: Clear selection before changing the font 2022-07-30 17:25 ` Helge Deller 2022-07-30 18:49 ` [PATCH] tty: vt: selection: Add check for valid tiocl_selection values Helge Deller @ 2022-07-30 18:50 ` Helge Deller 2022-07-31 11:32 ` Khalid Masum 2022-07-31 10:03 ` [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) Khalid Masum 2022-07-31 13:55 ` Khalid Masum 3 siblings, 1 reply; 28+ messages in thread From: Helge Deller @ 2022-07-30 18:50 UTC (permalink / raw) To: Khalid Masum, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby When changing the console font with ioctl(KDFONTOP) the new font size can be bigger than the previous font. A previous selection may thus now be outside of the new screen size and thus trigger out-of-bounds accesses to graphics memory if the selection is removed in vc_do_resize(). Prevent such out-of-memory accesses by dropping the selection before the various con_font_set() console handlers are called. Signed-off-by: Helge Deller <deller@gmx.de> Reported-by: syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index dfc1f4b445f3..3f09205185a4 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -4662,9 +4662,11 @@ static int con_font_set(struct vc_data *vc, struct console_font_op *op) console_lock(); if (vc->vc_mode != KD_TEXT) rc = -EINVAL; - else if (vc->vc_sw->con_font_set) + else if (vc->vc_sw->con_font_set) { + if (vc_is_sel(vc)) + clear_selection(); rc = vc->vc_sw->con_font_set(vc, &font, op->flags); - else + } else rc = -ENOSYS; console_unlock(); kfree(font.data); @@ -4691,9 +4693,11 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op) console_unlock(); return -EINVAL; } - if (vc->vc_sw->con_font_default) + if (vc->vc_sw->con_font_default) { + if (vc_is_sel(vc)) + clear_selection(); rc = vc->vc_sw->con_font_default(vc, &font, s); - else + } else rc = -ENOSYS; console_unlock(); if (!rc) { ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] vt: Clear selection before changing the font 2022-07-30 18:50 ` [PATCH] vt: Clear selection before changing the font Helge Deller @ 2022-07-31 11:32 ` Khalid Masum 0 siblings, 0 replies; 28+ messages in thread From: Khalid Masum @ 2022-07-31 11:32 UTC (permalink / raw) To: Helge Deller, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby On 7/31/22 00:50, Helge Deller wrote: > When changing the console font with ioctl(KDFONTOP) the new font size > can be bigger than the previous font. A previous selection may thus now > be outside of the new screen size and thus trigger out-of-bounds > accesses to graphics memory if the selection is removed in > vc_do_resize(). > > Prevent such out-of-memory accesses by dropping the selection before the > various con_font_set() console handlers are called. > > Signed-off-by: Helge Deller <deller@gmx.de> Tested-by: Khalid Masum <khalid.masum.92@gmail.com> > Reported-by: syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com > > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index dfc1f4b445f3..3f09205185a4 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -4662,9 +4662,11 @@ static int con_font_set(struct vc_data *vc, struct console_font_op *op) > console_lock(); > if (vc->vc_mode != KD_TEXT) > rc = -EINVAL; > - else if (vc->vc_sw->con_font_set) > + else if (vc->vc_sw->con_font_set) { > + if (vc_is_sel(vc)) > + clear_selection(); > rc = vc->vc_sw->con_font_set(vc, &font, op->flags); > - else > + } else > rc = -ENOSYS; > console_unlock(); > kfree(font.data); > @@ -4691,9 +4693,11 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op) > console_unlock(); > return -EINVAL; > } > - if (vc->vc_sw->con_font_default) > + if (vc->vc_sw->con_font_default) { > + if (vc_is_sel(vc)) > + clear_selection(); > rc = vc->vc_sw->con_font_default(vc, &font, s); > - else > + } else > rc = -ENOSYS; > console_unlock(); > if (!rc) { ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-07-30 17:25 ` Helge Deller 2022-07-30 18:49 ` [PATCH] tty: vt: selection: Add check for valid tiocl_selection values Helge Deller 2022-07-30 18:50 ` [PATCH] vt: Clear selection before changing the font Helge Deller @ 2022-07-31 10:03 ` Khalid Masum 2022-07-31 10:54 ` Helge Deller 2022-07-31 13:55 ` Khalid Masum 3 siblings, 1 reply; 28+ messages in thread From: Khalid Masum @ 2022-07-31 10:03 UTC (permalink / raw) To: Helge Deller, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby On 7/30/22 23:25, Helge Deller wrote: > On 7/29/22 08:51, Khalid Masum wrote: >> Here is a simplified reproducer for the issue: >> >> https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c > > The reproducer does this: Thanks for Looking into this. Being to this, so I have some questions. > ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3 ys:0 xe:0 ye:0 mode:0) = 0 How did you find out the selection values? From strace and man pages I know the third argument is an address. > -> sets the text selection area > ioctl(4, KDFONTOP) with op=0 (con_font_set), charcount=512 width=8 height=32, 0x20000000) = 0 Same here, It would be very helpful if you could tell me how. > -> changes the font size. > > It does not crash with current Linus' head (v5.19-rc8). I tested in 5.19-rc8 in Qemu x86_64 and it crashed for me. > Kernel v5.16, which was used by this KASAN report, hasn't received backports > since months, so I tried stable kernel v5.15.58 instead, and this > kernel crashed with the reproducer. > > The reproducer brings up two issues with current code: > 1. The reproducer uses ioctl(TIOCLINUX, TIOCL_SETSEL) and hands over (invalid) > zero-values for ys and ye for the starting lines. > This is wrong, since the API seems to expect a "1" as the very first line for the selection. > This can be easily fixed by adding checks for zero-values and return -EINVAL if found. > > But this bug isn't critical itself and is not the reason for the kernel crash. > Without the checks, the ioctl handler simply wraps the coordinate values and converts them > from: > input selection: xs:3 ys:0 xe:0 ye:0 mode:0 to the new: > vc_selection = xs:2 ys:23 xe:127 ye:23 mode:0 > which is the current maximum coordinates for the screen. > > Those higher values now trigger issue #2: > After the TIOCL_SETSEL the last line on the screen is now selected. The KDFONTOP ioctl > then sets a 8x32 console font, and replaces the former 8x16 console font. > With the bigger font the current screen selection is now outside the visible screen > and this finally triggeres this backtrace, because vc_do_resize() calls clear_selection() > to unhighlight the selection (which starts to render chars outside of the screen): That makes sense. > drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:794 [inline] > drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2288 > bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:124 [inline] > bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:173 > fbcon_putcs+0x353/0x440 drivers/video/fbdev/core/fbcon.c:1277 > do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 > invert_screen+0x1d4/0x600 drivers/tty/vt/vt.c:800 > highlight drivers/tty/vt/selection.c:57 [inline] > clear_selection drivers/tty/vt/selection.c:84 [inline] > clear_selection+0x55/0x70 drivers/tty/vt/selection.c:80 > vc_do_resize+0xe6e/0x1180 drivers/tty/vt/vt.c:1257 > > IMHO the easiest way to prevent this crash is to simply clear the > selection before the various con_font_set() console handlers are called. > Otherwise every console driver needs to add checks and verify if the current > selection still fits with the selected font, which gets tricky because some > of those drivers fiddle with the screen width&height before calling vc_do_resize(). > > I'll follow up to this mail with patches for both issues shortly. I tested the patches. The crash no longer occurs with the reproducer. > Helge Thanks, -- Khalid Masum ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-07-31 10:03 ` [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) Khalid Masum @ 2022-07-31 10:54 ` Helge Deller 0 siblings, 0 replies; 28+ messages in thread From: Helge Deller @ 2022-07-31 10:54 UTC (permalink / raw) To: Khalid Masum Cc: Helge Deller, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby * Khalid Masum <khalid.masum.92@gmail.com>: > On 7/30/22 23:25, Helge Deller wrote: > > On 7/29/22 08:51, Khalid Masum wrote: > > > Here is a simplified reproducer for the issue: > > > > > > https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c > > > > The reproducer does this: > > Thanks for Looking into this. Being to this, so I have some questions. > > ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3 ys:0 xe:0 ye:0 mode:0) = 0 > > How did you find out the selection values? From strace and man pages I know > the third argument is an address. Right. It's a pointer into userspace. I simply added printk debug code to see what's happening. I've attached that patch below. > > -> sets the text selection area > > ioctl(4, KDFONTOP) with op=0 (con_font_set), charcount=512 width=8 height=32, 0x20000000) = 0 > > Same here, It would be very helpful if you could tell me how. See patch below. > > -> changes the font size. > > > > It does not crash with current Linus' head (v5.19-rc8). > > I tested in 5.19-rc8 in Qemu x86_64 and it crashed for me. That's strange, since I tested the same. Maybe I did something wrong. Anyway, the patches I sent applies to all kernel versions. > > Kernel v5.16, which was used by this KASAN report, hasn't received backports > > since months, so I tried stable kernel v5.15.58 instead, and this > > kernel crashed with the reproducer. > > > > The reproducer brings up two issues with current code: > > 1. The reproducer uses ioctl(TIOCLINUX, TIOCL_SETSEL) and hands over (invalid) > > zero-values for ys and ye for the starting lines. > > This is wrong, since the API seems to expect a "1" as the very first line for the selection. > > This can be easily fixed by adding checks for zero-values and return -EINVAL if found. > > > > But this bug isn't critical itself and is not the reason for the kernel crash. > > Without the checks, the ioctl handler simply wraps the coordinate values and converts them > > from: > > input selection: xs:3 ys:0 xe:0 ye:0 mode:0 to the new: > > vc_selection = xs:2 ys:23 xe:127 ye:23 mode:0 > > which is the current maximum coordinates for the screen. > > > > Those higher values now trigger issue #2: > > After the TIOCL_SETSEL the last line on the screen is now selected. The KDFONTOP ioctl > > then sets a 8x32 console font, and replaces the former 8x16 console font. > > With the bigger font the current screen selection is now outside the visible screen > > and this finally triggeres this backtrace, because vc_do_resize() calls clear_selection() > > to unhighlight the selection (which starts to render chars outside of the screen): > > That makes sense. > > > drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:794 [inline] > > drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2288 > > bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:124 [inline] > > bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:173 > > fbcon_putcs+0x353/0x440 drivers/video/fbdev/core/fbcon.c:1277 > > do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 > > invert_screen+0x1d4/0x600 drivers/tty/vt/vt.c:800 > > highlight drivers/tty/vt/selection.c:57 [inline] > > clear_selection drivers/tty/vt/selection.c:84 [inline] > > clear_selection+0x55/0x70 drivers/tty/vt/selection.c:80 > > vc_do_resize+0xe6e/0x1180 drivers/tty/vt/vt.c:1257 > > > > IMHO the easiest way to prevent this crash is to simply clear the > > selection before the various con_font_set() console handlers are called. > > Otherwise every console driver needs to add checks and verify if the current > > selection still fits with the selected font, which gets tricky because some > > of those drivers fiddle with the screen width&height before calling vc_do_resize(). > > > > I'll follow up to this mail with patches for both issues shortly. > > I tested the patches. The crash no longer occurs with the reproducer. Thanks for testing! Maybe you want to reply to the patches with a Tested-by: tag? Below is my debug code. Helge diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index 58692a9b4097..0167b368a70f 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -333,6 +333,7 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); v->ye = min_t(u16, v->ye - 1, vc->vc_rows - 1); + printk("vc_selection = xs:%u ys:%u xe:%u ye:%u mode:%u\n", v->xs, v->ys, v->xe, v->ye, v->sel_mode); if (mouse_reporting() && (v->sel_mode & TIOCL_SELMOUSEREPORT)) { mouse_report(tty, v->sel_mode & TIOCL_SELBUTTONMASK, v->xs, @@ -357,6 +358,7 @@ int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty) { int ret; + printk("tiocl_selection = xs:%u ys:%u xe:%u ye:%u mode:%u\n", v->xs, v->ys, v->xe, v->ye, v->sel_mode); mutex_lock(&vc_sel.lock); console_lock(); ret = vc_selection(vc_cons[fg_console].d, v, tty); diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 3f09205185a4..a0b4570c959a 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -3194,6 +3194,8 @@ int tioclinux(struct tty_struct *tty, unsigned long arg) return -EFAULT; ret = 0; + printk("tioclinux: type = %d\n", type); // TIOCL_SETSEL + switch (type) { case TIOCL_SETSEL: @@ -4655,6 +4657,8 @@ static int con_font_set(struct vc_data *vc, struct console_font_op *op) if (IS_ERR(font.data)) return PTR_ERR(font.data); + pr_err("con_font_set charcount %d w:%d h:%d\n", op->charcount, op->width, op->height); + font.charcount = op->charcount; font.width = op->width; font.height = op->height; @@ -4709,6 +4713,7 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op) int con_font_op(struct vc_data *vc, struct console_font_op *op) { + pr_warn("con_font_op op = %d\n", op->op); switch (op->op) { case KD_FONT_OP_SET: return con_font_set(vc, op); ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) @ 2022-07-31 10:54 ` Helge Deller 0 siblings, 0 replies; 28+ messages in thread From: Helge Deller @ 2022-07-31 10:54 UTC (permalink / raw) To: Khalid Masum Cc: linux-fbdev, Greg Kroah-Hartman, Helge Deller, syzkaller-bugs, linux-kernel, dri-devel, syzbot, Jiri Slaby * Khalid Masum <khalid.masum.92@gmail.com>: > On 7/30/22 23:25, Helge Deller wrote: > > On 7/29/22 08:51, Khalid Masum wrote: > > > Here is a simplified reproducer for the issue: > > > > > > https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c > > > > The reproducer does this: > > Thanks for Looking into this. Being to this, so I have some questions. > > ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3 ys:0 xe:0 ye:0 mode:0) = 0 > > How did you find out the selection values? From strace and man pages I know > the third argument is an address. Right. It's a pointer into userspace. I simply added printk debug code to see what's happening. I've attached that patch below. > > -> sets the text selection area > > ioctl(4, KDFONTOP) with op=0 (con_font_set), charcount=512 width=8 height=32, 0x20000000) = 0 > > Same here, It would be very helpful if you could tell me how. See patch below. > > -> changes the font size. > > > > It does not crash with current Linus' head (v5.19-rc8). > > I tested in 5.19-rc8 in Qemu x86_64 and it crashed for me. That's strange, since I tested the same. Maybe I did something wrong. Anyway, the patches I sent applies to all kernel versions. > > Kernel v5.16, which was used by this KASAN report, hasn't received backports > > since months, so I tried stable kernel v5.15.58 instead, and this > > kernel crashed with the reproducer. > > > > The reproducer brings up two issues with current code: > > 1. The reproducer uses ioctl(TIOCLINUX, TIOCL_SETSEL) and hands over (invalid) > > zero-values for ys and ye for the starting lines. > > This is wrong, since the API seems to expect a "1" as the very first line for the selection. > > This can be easily fixed by adding checks for zero-values and return -EINVAL if found. > > > > But this bug isn't critical itself and is not the reason for the kernel crash. > > Without the checks, the ioctl handler simply wraps the coordinate values and converts them > > from: > > input selection: xs:3 ys:0 xe:0 ye:0 mode:0 to the new: > > vc_selection = xs:2 ys:23 xe:127 ye:23 mode:0 > > which is the current maximum coordinates for the screen. > > > > Those higher values now trigger issue #2: > > After the TIOCL_SETSEL the last line on the screen is now selected. The KDFONTOP ioctl > > then sets a 8x32 console font, and replaces the former 8x16 console font. > > With the bigger font the current screen selection is now outside the visible screen > > and this finally triggeres this backtrace, because vc_do_resize() calls clear_selection() > > to unhighlight the selection (which starts to render chars outside of the screen): > > That makes sense. > > > drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:794 [inline] > > drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2288 > > bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:124 [inline] > > bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:173 > > fbcon_putcs+0x353/0x440 drivers/video/fbdev/core/fbcon.c:1277 > > do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 > > invert_screen+0x1d4/0x600 drivers/tty/vt/vt.c:800 > > highlight drivers/tty/vt/selection.c:57 [inline] > > clear_selection drivers/tty/vt/selection.c:84 [inline] > > clear_selection+0x55/0x70 drivers/tty/vt/selection.c:80 > > vc_do_resize+0xe6e/0x1180 drivers/tty/vt/vt.c:1257 > > > > IMHO the easiest way to prevent this crash is to simply clear the > > selection before the various con_font_set() console handlers are called. > > Otherwise every console driver needs to add checks and verify if the current > > selection still fits with the selected font, which gets tricky because some > > of those drivers fiddle with the screen width&height before calling vc_do_resize(). > > > > I'll follow up to this mail with patches for both issues shortly. > > I tested the patches. The crash no longer occurs with the reproducer. Thanks for testing! Maybe you want to reply to the patches with a Tested-by: tag? Below is my debug code. Helge diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index 58692a9b4097..0167b368a70f 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -333,6 +333,7 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); v->ye = min_t(u16, v->ye - 1, vc->vc_rows - 1); + printk("vc_selection = xs:%u ys:%u xe:%u ye:%u mode:%u\n", v->xs, v->ys, v->xe, v->ye, v->sel_mode); if (mouse_reporting() && (v->sel_mode & TIOCL_SELMOUSEREPORT)) { mouse_report(tty, v->sel_mode & TIOCL_SELBUTTONMASK, v->xs, @@ -357,6 +358,7 @@ int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty) { int ret; + printk("tiocl_selection = xs:%u ys:%u xe:%u ye:%u mode:%u\n", v->xs, v->ys, v->xe, v->ye, v->sel_mode); mutex_lock(&vc_sel.lock); console_lock(); ret = vc_selection(vc_cons[fg_console].d, v, tty); diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 3f09205185a4..a0b4570c959a 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -3194,6 +3194,8 @@ int tioclinux(struct tty_struct *tty, unsigned long arg) return -EFAULT; ret = 0; + printk("tioclinux: type = %d\n", type); // TIOCL_SETSEL + switch (type) { case TIOCL_SETSEL: @@ -4655,6 +4657,8 @@ static int con_font_set(struct vc_data *vc, struct console_font_op *op) if (IS_ERR(font.data)) return PTR_ERR(font.data); + pr_err("con_font_set charcount %d w:%d h:%d\n", op->charcount, op->width, op->height); + font.charcount = op->charcount; font.width = op->width; font.height = op->height; @@ -4709,6 +4713,7 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op) int con_font_op(struct vc_data *vc, struct console_font_op *op) { + pr_warn("con_font_op op = %d\n", op->op); switch (op->op) { case KD_FONT_OP_SET: return con_font_set(vc, op); ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-07-31 10:54 ` Helge Deller @ 2022-07-31 11:23 ` Khalid Masum -1 siblings, 0 replies; 28+ messages in thread From: Khalid Masum @ 2022-07-31 11:23 UTC (permalink / raw) To: Helge Deller Cc: syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby On 7/31/22 16:54, Helge Deller wrote: > * Khalid Masum <khalid.masum.92@gmail.com>: >> On 7/30/22 23:25, Helge Deller wrote: >>> On 7/29/22 08:51, Khalid Masum wrote: >>>> Here is a simplified reproducer for the issue: >>>> >>>> https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c >>> >>> The reproducer does this: >> >> Thanks for Looking into this. Being to this, so I have some questions. >>> ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3 ys:0 xe:0 ye:0 mode:0) = 0 >> >> How did you find out the selection values? From strace and man pages I know >> the third argument is an address. > > Right. It's a pointer into userspace. > I simply added printk debug code to see what's happening. > I've attached that patch below. > > >>> -> sets the text selection area >>> ioctl(4, KDFONTOP) with op=0 (con_font_set), charcount=512 width=8 height=32, 0x20000000) = 0 >> >> Same here, It would be very helpful if you could tell me how. > > See patch below. > >>> -> changes the font size. >>> >>> It does not crash with current Linus' head (v5.19-rc8). >> >> I tested in 5.19-rc8 in Qemu x86_64 and it crashed for me. > > That's strange, since I tested the same. Maybe I did something wrong. > Anyway, the patches I sent applies to all kernel versions. > >>> Kernel v5.16, which was used by this KASAN report, hasn't received backports >>> since months, so I tried stable kernel v5.15.58 instead, and this >>> kernel crashed with the reproducer. >>> >>> The reproducer brings up two issues with current code: >>> 1. The reproducer uses ioctl(TIOCLINUX, TIOCL_SETSEL) and hands over (invalid) >>> zero-values for ys and ye for the starting lines. >>> This is wrong, since the API seems to expect a "1" as the very first line for the selection. >>> This can be easily fixed by adding checks for zero-values and return -EINVAL if found. >>> >>> But this bug isn't critical itself and is not the reason for the kernel crash. >>> Without the checks, the ioctl handler simply wraps the coordinate values and converts them >>> from: >>> input selection: xs:3 ys:0 xe:0 ye:0 mode:0 to the new: >>> vc_selection = xs:2 ys:23 xe:127 ye:23 mode:0 >>> which is the current maximum coordinates for the screen. >>> >>> Those higher values now trigger issue #2: >>> After the TIOCL_SETSEL the last line on the screen is now selected. The KDFONTOP ioctl >>> then sets a 8x32 console font, and replaces the former 8x16 console font. >>> With the bigger font the current screen selection is now outside the visible screen >>> and this finally triggeres this backtrace, because vc_do_resize() calls clear_selection() >>> to unhighlight the selection (which starts to render chars outside of the screen): >> >> That makes sense. >> >>> drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:794 [inline] >>> drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2288 >>> bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:124 [inline] >>> bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:173 >>> fbcon_putcs+0x353/0x440 drivers/video/fbdev/core/fbcon.c:1277 >>> do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 >>> invert_screen+0x1d4/0x600 drivers/tty/vt/vt.c:800 >>> highlight drivers/tty/vt/selection.c:57 [inline] >>> clear_selection drivers/tty/vt/selection.c:84 [inline] >>> clear_selection+0x55/0x70 drivers/tty/vt/selection.c:80 >>> vc_do_resize+0xe6e/0x1180 drivers/tty/vt/vt.c:1257 >>> >>> IMHO the easiest way to prevent this crash is to simply clear the >>> selection before the various con_font_set() console handlers are called. >>> Otherwise every console driver needs to add checks and verify if the current >>> selection still fits with the selected font, which gets tricky because some >>> of those drivers fiddle with the screen width&height before calling vc_do_resize(). >>> >>> I'll follow up to this mail with patches for both issues shortly. >> >> I tested the patches. The crash no longer occurs with the reproducer. > > Thanks for testing! > Maybe you want to reply to the patches with a Tested-by: tag? Sure, I will do that. > > Below is my debug code. Thanks for the debug code! It explains a lot. > Helge > > > diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c > index 58692a9b4097..0167b368a70f 100644 > --- a/drivers/tty/vt/selection.c > +++ b/drivers/tty/vt/selection.c > @@ -333,6 +333,7 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, > v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); > v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); > v->ye = min_t(u16, v->ye - 1, vc->vc_rows - 1); > + printk("vc_selection = xs:%u ys:%u xe:%u ye:%u mode:%u\n", v->xs, v->ys, v->xe, v->ye, v->sel_mode); > > if (mouse_reporting() && (v->sel_mode & TIOCL_SELMOUSEREPORT)) { > mouse_report(tty, v->sel_mode & TIOCL_SELBUTTONMASK, v->xs, > @@ -357,6 +358,7 @@ int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty) > { > int ret; > > + printk("tiocl_selection = xs:%u ys:%u xe:%u ye:%u mode:%u\n", v->xs, v->ys, v->xe, v->ye, v->sel_mode); > mutex_lock(&vc_sel.lock); > console_lock(); > ret = vc_selection(vc_cons[fg_console].d, v, tty); > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index 3f09205185a4..a0b4570c959a 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -3194,6 +3194,8 @@ int tioclinux(struct tty_struct *tty, unsigned long arg) > return -EFAULT; > ret = 0; > > + printk("tioclinux: type = %d\n", type); // TIOCL_SETSEL > + > switch (type) > { > case TIOCL_SETSEL: > @@ -4655,6 +4657,8 @@ static int con_font_set(struct vc_data *vc, struct console_font_op *op) > if (IS_ERR(font.data)) > return PTR_ERR(font.data); > > + pr_err("con_font_set charcount %d w:%d h:%d\n", op->charcount, op->width, op->height); > + > font.charcount = op->charcount; > font.width = op->width; > font.height = op->height; > @@ -4709,6 +4713,7 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op) > > int con_font_op(struct vc_data *vc, struct console_font_op *op) > { > + pr_warn("con_font_op op = %d\n", op->op); > switch (op->op) { > case KD_FONT_OP_SET: > return con_font_set(vc, op); Thanks, -- Khalid Masum ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) @ 2022-07-31 11:23 ` Khalid Masum 0 siblings, 0 replies; 28+ messages in thread From: Khalid Masum @ 2022-07-31 11:23 UTC (permalink / raw) To: Helge Deller Cc: linux-fbdev, Greg Kroah-Hartman, syzkaller-bugs, linux-kernel, dri-devel, syzbot, Jiri Slaby On 7/31/22 16:54, Helge Deller wrote: > * Khalid Masum <khalid.masum.92@gmail.com>: >> On 7/30/22 23:25, Helge Deller wrote: >>> On 7/29/22 08:51, Khalid Masum wrote: >>>> Here is a simplified reproducer for the issue: >>>> >>>> https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c >>> >>> The reproducer does this: >> >> Thanks for Looking into this. Being to this, so I have some questions. >>> ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3 ys:0 xe:0 ye:0 mode:0) = 0 >> >> How did you find out the selection values? From strace and man pages I know >> the third argument is an address. > > Right. It's a pointer into userspace. > I simply added printk debug code to see what's happening. > I've attached that patch below. > > >>> -> sets the text selection area >>> ioctl(4, KDFONTOP) with op=0 (con_font_set), charcount=512 width=8 height=32, 0x20000000) = 0 >> >> Same here, It would be very helpful if you could tell me how. > > See patch below. > >>> -> changes the font size. >>> >>> It does not crash with current Linus' head (v5.19-rc8). >> >> I tested in 5.19-rc8 in Qemu x86_64 and it crashed for me. > > That's strange, since I tested the same. Maybe I did something wrong. > Anyway, the patches I sent applies to all kernel versions. > >>> Kernel v5.16, which was used by this KASAN report, hasn't received backports >>> since months, so I tried stable kernel v5.15.58 instead, and this >>> kernel crashed with the reproducer. >>> >>> The reproducer brings up two issues with current code: >>> 1. The reproducer uses ioctl(TIOCLINUX, TIOCL_SETSEL) and hands over (invalid) >>> zero-values for ys and ye for the starting lines. >>> This is wrong, since the API seems to expect a "1" as the very first line for the selection. >>> This can be easily fixed by adding checks for zero-values and return -EINVAL if found. >>> >>> But this bug isn't critical itself and is not the reason for the kernel crash. >>> Without the checks, the ioctl handler simply wraps the coordinate values and converts them >>> from: >>> input selection: xs:3 ys:0 xe:0 ye:0 mode:0 to the new: >>> vc_selection = xs:2 ys:23 xe:127 ye:23 mode:0 >>> which is the current maximum coordinates for the screen. >>> >>> Those higher values now trigger issue #2: >>> After the TIOCL_SETSEL the last line on the screen is now selected. The KDFONTOP ioctl >>> then sets a 8x32 console font, and replaces the former 8x16 console font. >>> With the bigger font the current screen selection is now outside the visible screen >>> and this finally triggeres this backtrace, because vc_do_resize() calls clear_selection() >>> to unhighlight the selection (which starts to render chars outside of the screen): >> >> That makes sense. >> >>> drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:794 [inline] >>> drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2288 >>> bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:124 [inline] >>> bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:173 >>> fbcon_putcs+0x353/0x440 drivers/video/fbdev/core/fbcon.c:1277 >>> do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 >>> invert_screen+0x1d4/0x600 drivers/tty/vt/vt.c:800 >>> highlight drivers/tty/vt/selection.c:57 [inline] >>> clear_selection drivers/tty/vt/selection.c:84 [inline] >>> clear_selection+0x55/0x70 drivers/tty/vt/selection.c:80 >>> vc_do_resize+0xe6e/0x1180 drivers/tty/vt/vt.c:1257 >>> >>> IMHO the easiest way to prevent this crash is to simply clear the >>> selection before the various con_font_set() console handlers are called. >>> Otherwise every console driver needs to add checks and verify if the current >>> selection still fits with the selected font, which gets tricky because some >>> of those drivers fiddle with the screen width&height before calling vc_do_resize(). >>> >>> I'll follow up to this mail with patches for both issues shortly. >> >> I tested the patches. The crash no longer occurs with the reproducer. > > Thanks for testing! > Maybe you want to reply to the patches with a Tested-by: tag? Sure, I will do that. > > Below is my debug code. Thanks for the debug code! It explains a lot. > Helge > > > diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c > index 58692a9b4097..0167b368a70f 100644 > --- a/drivers/tty/vt/selection.c > +++ b/drivers/tty/vt/selection.c > @@ -333,6 +333,7 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, > v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); > v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); > v->ye = min_t(u16, v->ye - 1, vc->vc_rows - 1); > + printk("vc_selection = xs:%u ys:%u xe:%u ye:%u mode:%u\n", v->xs, v->ys, v->xe, v->ye, v->sel_mode); > > if (mouse_reporting() && (v->sel_mode & TIOCL_SELMOUSEREPORT)) { > mouse_report(tty, v->sel_mode & TIOCL_SELBUTTONMASK, v->xs, > @@ -357,6 +358,7 @@ int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty) > { > int ret; > > + printk("tiocl_selection = xs:%u ys:%u xe:%u ye:%u mode:%u\n", v->xs, v->ys, v->xe, v->ye, v->sel_mode); > mutex_lock(&vc_sel.lock); > console_lock(); > ret = vc_selection(vc_cons[fg_console].d, v, tty); > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index 3f09205185a4..a0b4570c959a 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -3194,6 +3194,8 @@ int tioclinux(struct tty_struct *tty, unsigned long arg) > return -EFAULT; > ret = 0; > > + printk("tioclinux: type = %d\n", type); // TIOCL_SETSEL > + > switch (type) > { > case TIOCL_SETSEL: > @@ -4655,6 +4657,8 @@ static int con_font_set(struct vc_data *vc, struct console_font_op *op) > if (IS_ERR(font.data)) > return PTR_ERR(font.data); > > + pr_err("con_font_set charcount %d w:%d h:%d\n", op->charcount, op->width, op->height); > + > font.charcount = op->charcount; > font.width = op->width; > font.height = op->height; > @@ -4709,6 +4713,7 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op) > > int con_font_op(struct vc_data *vc, struct console_font_op *op) > { > + pr_warn("con_font_op op = %d\n", op->op); > switch (op->op) { > case KD_FONT_OP_SET: > return con_font_set(vc, op); Thanks, -- Khalid Masum ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-07-30 17:25 ` Helge Deller ` (2 preceding siblings ...) 2022-07-31 10:03 ` [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) Khalid Masum @ 2022-07-31 13:55 ` Khalid Masum 2022-07-31 15:39 ` Helge Deller 3 siblings, 1 reply; 28+ messages in thread From: Khalid Masum @ 2022-07-31 13:55 UTC (permalink / raw) To: Helge Deller, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby On 7/30/22 23:25, Helge Deller wrote: > On 7/29/22 08:51, Khalid Masum wrote: >> Here is a simplified reproducer for the issue: >> >> https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c > > The reproducer does this: > ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3 ys:0 xe:0 ye:0 mode:0) = 0 > -> sets the text selection area > ioctl(4, KDFONTOP) with op=0 (con_font_set), charcount=512 width=8 height=32, 0x20000000) = 0 > -> changes the font size. > > It does not crash with current Linus' head (v5.19-rc8). > Kernel v5.16, which was used by this KASAN report, hasn't received backports > since months, so I tried stable kernel v5.15.58 instead, and this > kernel crashed with the reproducer. > > The reproducer brings up two issues with current code: > 1. The reproducer uses ioctl(TIOCLINUX, TIOCL_SETSEL) and hands over (invalid) > zero-values for ys and ye for the starting lines. > This is wrong, since the API seems to expect a "1" as the very first line for the selection. Why do you think that API expects a 1? > This can be easily fixed by adding checks for zero-values and return -EINVAL if found. > > But this bug isn't critical itself and is not the reason for the kernel crash. > Without the checks, the ioctl handler simply wraps the coordinate values and converts them > from: > input selection: xs:3 ys:0 xe:0 ye:0 mode:0 to the new: > vc_selection = xs:2 ys:23 xe:127 ye:23 mode:0 > which is the current maximum coordinates for the screen. > > Those higher values now trigger issue #2: > After the TIOCL_SETSEL the last line on the screen is now selected. The KDFONTOP ioctl > then sets a 8x32 console font, and replaces the former 8x16 console font. > With the bigger font the current screen selection is now outside the visible screen > and this finally triggeres this backtrace, because vc_do_resize() calls clear_selection() > to unhighlight the selection (which starts to render chars outside of the screen): > > drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:794 [inline] > drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2288 > bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:124 [inline] > bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:173 > fbcon_putcs+0x353/0x440 drivers/video/fbdev/core/fbcon.c:1277 > do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 > invert_screen+0x1d4/0x600 drivers/tty/vt/vt.c:800 > highlight drivers/tty/vt/selection.c:57 [inline] > clear_selection drivers/tty/vt/selection.c:84 [inline] > clear_selection+0x55/0x70 drivers/tty/vt/selection.c:80 > vc_do_resize+0xe6e/0x1180 drivers/tty/vt/vt.c:1257 > > IMHO the easiest way to prevent this crash is to simply clear the > selection before the various con_font_set() console handlers are called. > Otherwise every console driver needs to add checks and verify if the current > selection still fits with the selected font, which gets tricky because some > of those drivers fiddle with the screen width&height before calling vc_do_resize(). > > I'll follow up to this mail with patches for both issues shortly. > > Helge Thanks, -- Khalid Masum ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-07-31 13:55 ` Khalid Masum @ 2022-07-31 15:39 ` Helge Deller 2022-08-01 4:09 ` Khalid Masum 0 siblings, 1 reply; 28+ messages in thread From: Helge Deller @ 2022-07-31 15:39 UTC (permalink / raw) To: Khalid Masum, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby On 7/31/22 15:55, Khalid Masum wrote: > On 7/30/22 23:25, Helge Deller wrote: >> On 7/29/22 08:51, Khalid Masum wrote: >>> Here is a simplified reproducer for the issue: >>> >>> https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c >> >> The reproducer does this: >> ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3 ys:0 xe:0 ye:0 mode:0) = 0 >> -> sets the text selection area >> ioctl(4, KDFONTOP) with op=0 (con_font_set), charcount=512 width=8 height=32, 0x20000000) = 0 >> -> changes the font size. >> >> It does not crash with current Linus' head (v5.19-rc8). >> Kernel v5.16, which was used by this KASAN report, hasn't received backports >> since months, so I tried stable kernel v5.15.58 instead, and this >> kernel crashed with the reproducer. >> >> The reproducer brings up two issues with current code: >> 1. The reproducer uses ioctl(TIOCLINUX, TIOCL_SETSEL) and hands over (invalid) >> zero-values for ys and ye for the starting lines. >> This is wrong, since the API seems to expect a "1" as the very first line for the selection. > > Why do you think that API expects a 1? because the code in drivers/tty/vt/selection.c indicates this: See: static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, struct tty_struct *tty) { int ps, pe; ... v->xs = min_t(u16, v->xs - 1, vc->vc_cols - 1); v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); v->ye = min_t(u16, v->ye - 1, vc->vc_rows - 1); ... ps = v->ys * vc->vc_size_row + (v->xs << 1); pe = v->ye * vc->vc_size_row + (v->xe << 1); The userspace-provided xs,ys,xe and ye values are decremented by one before ps and pe (the pointers into the text screen array) are calculated. So, for the xs...ye values in the range from 1..max-screen-lines/columns are expected. Helge ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-07-31 15:39 ` Helge Deller @ 2022-08-01 4:09 ` Khalid Masum 0 siblings, 0 replies; 28+ messages in thread From: Khalid Masum @ 2022-08-01 4:09 UTC (permalink / raw) To: Helge Deller, syzbot, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby On 7/31/22 21:39, Helge Deller wrote: > On 7/31/22 15:55, Khalid Masum wrote: >> On 7/30/22 23:25, Helge Deller wrote: >>> On 7/29/22 08:51, Khalid Masum wrote: >>>> Here is a simplified reproducer for the issue: >>>> >>>> https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c >>> >>> The reproducer does this: >>> ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3 ys:0 xe:0 ye:0 mode:0) = 0 >>> -> sets the text selection area >>> ioctl(4, KDFONTOP) with op=0 (con_font_set), charcount=512 width=8 height=32, 0x20000000) = 0 >>> -> changes the font size. >>> >>> It does not crash with current Linus' head (v5.19-rc8). >>> Kernel v5.16, which was used by this KASAN report, hasn't received backports >>> since months, so I tried stable kernel v5.15.58 instead, and this >>> kernel crashed with the reproducer. >>> >>> The reproducer brings up two issues with current code: >>> 1. The reproducer uses ioctl(TIOCLINUX, TIOCL_SETSEL) and hands over (invalid) >>> zero-values for ys and ye for the starting lines. >>> This is wrong, since the API seems to expect a "1" as the very first line for the selection. >> >> Why do you think that API expects a 1? > > because the code in drivers/tty/vt/selection.c indicates this: > See: > static int vc_selection(struct vc_data *vc, struct tiocl_selection *v, struct tty_struct *tty) > { > int ps, pe; > ... > v->xs = min_t(u16, v->xs - 1, vc->vc_cols - 1); > v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1); > v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1); > v->ye = min_t(u16, v->ye - 1, vc->vc_rows - 1); > ... > ps = v->ys * vc->vc_size_row + (v->xs << 1); > pe = v->ye * vc->vc_size_row + (v->xe << 1); > > The userspace-provided xs,ys,xe and ye values are decremented by one > before ps and pe (the pointers into the text screen array) are calculated. > So, for the xs...ye values in the range from 1..max-screen-lines/columns > are expected. Oh. I got it. It is pretty simple. Thanks again for explaining. > > Helge Khalid Masum ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2021-11-19 9:18 [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) syzbot ` (2 preceding siblings ...) 2022-07-29 6:51 ` Khalid Masum @ 2022-07-30 8:12 ` Khalid Masum 2022-07-30 10:55 ` syzbot 2022-08-01 10:43 ` Dan Carpenter 2022-07-30 11:45 ` Khalid Masum 4 siblings, 2 replies; 28+ messages in thread From: Khalid Masum @ 2022-07-30 8:12 UTC (permalink / raw) To: linux-kernel, syzkaller-bugs, syzbot+14b0e8f3fd1612e35350 Cc: khalid.masum.92, khalid.masum Currently the if block's condition has an unhandled case, where the result of ret might get greater than vc->vc_scr_end, and therefore the corresponding handler in else block never gets executed. Which eventually causes panic in fast_imageblit. Add this extra check in the conditions to fix this breakage. #syz-test: https://github.com/torvalds/linux.git e0dccc3b76fb --- drivers/video/fbdev/core/fbcon.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 1a9aa12cf886..d026f3845b60 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -2591,14 +2591,13 @@ static unsigned long fbcon_getxy(struct vc_data *vc, unsigned long pos, { unsigned long ret; int x, y; + unsigned long offset = (pos - vc->vc_origin) / 2; + x = offset % vc->vc_cols; + y = offset / vc->vc_cols; + ret = pos + (vc->vc_cols - x) * 2; - if (pos >= vc->vc_origin && pos < vc->vc_scr_end) { - unsigned long offset = (pos - vc->vc_origin) / 2; - - x = offset % vc->vc_cols; - y = offset / vc->vc_cols; - ret = pos + (vc->vc_cols - x) * 2; - } else { + if (!pos >= vc->vc_origin || !pos < vc->vc_scr_end || + !ret < vc->vc_scr_end) { /* Should not happen */ x = y = 0; ret = vc->vc_origin; -- 2.36.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-07-30 8:12 ` Khalid Masum @ 2022-07-30 10:55 ` syzbot 2022-08-01 10:43 ` Dan Carpenter 1 sibling, 0 replies; 28+ messages in thread From: syzbot @ 2022-07-30 10:55 UTC (permalink / raw) To: khalid.masum.92, khalid.masum, linux-kernel, syzkaller-bugs Hello, syzbot has tested the proposed patch but the reproducer is still triggering an issue: KASAN: vmalloc-out-of-bounds Write in imageblit ================================================================== BUG: KASAN: vmalloc-out-of-bounds in fast_imageblit drivers/video/fbdev/core/sysimgblt.c:257 [inline] BUG: KASAN: vmalloc-out-of-bounds in sys_imageblit+0x1ed0/0x2240 drivers/video/fbdev/core/sysimgblt.c:323 Write of size 4 at addr ffffc90004301000 by task syz-executor.3/4204 CPU: 1 PID: 4204 Comm: syz-executor.3 Not tainted 5.19.0-rc8-syzkaller-dirty #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/22/2022 Call Trace: <TASK> __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106 print_address_description.constprop.0.cold+0xf/0x495 mm/kasan/report.c:313 print_report mm/kasan/report.c:429 [inline] kasan_report.cold+0xf4/0x1c6 mm/kasan/report.c:491 fast_imageblit drivers/video/fbdev/core/sysimgblt.c:257 [inline] sys_imageblit+0x1ed0/0x2240 drivers/video/fbdev/core/sysimgblt.c:323 drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:825 [inline] drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2328 bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:139 [inline] bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:188 fbcon_putcs+0x314/0x3e0 drivers/video/fbdev/core/fbcon.c:1285 do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 redraw_screen+0x61f/0x740 drivers/tty/vt/vt.c:1035 fbcon_do_set_font+0x5eb/0x6f0 drivers/video/fbdev/core/fbcon.c:2435 fbcon_set_font+0x89d/0xab0 drivers/video/fbdev/core/fbcon.c:2522 con_font_set drivers/tty/vt/vt.c:4666 [inline] con_font_op+0x73a/0xc90 drivers/tty/vt/vt.c:4710 vt_k_ioctl drivers/tty/vt/vt_ioctl.c:474 [inline] vt_ioctl+0x1efa/0x2b20 drivers/tty/vt/vt_ioctl.c:752 tty_ioctl+0xbbd/0x15e0 drivers/tty/tty_io.c:2778 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:870 [inline] __se_sys_ioctl fs/ioctl.c:856 [inline] __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:856 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x7f57f9089109 Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 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 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007f57fa20f168 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 00007f57f919bf60 RCX: 00007f57f9089109 RDX: 0000000020000040 RSI: 0000000000004b72 RDI: 0000000000000004 RBP: 00007f57fa20f1d0 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001 R13: 00007fffa57859ef R14: 00007f57fa20f300 R15: 0000000000022000 </TASK> The buggy address belongs to the virtual mapping at [ffffc90004001000, ffffc90004302000) created by: drm_gem_shmem_vmap_locked drivers/gpu/drm/drm_gem_shmem_helper.c:319 [inline] drm_gem_shmem_vmap+0x3d7/0x5a0 drivers/gpu/drm/drm_gem_shmem_helper.c:366 Memory state around the buggy address: ffffc90004300f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ffffc90004300f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >ffffc90004301000: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 ^ ffffc90004301080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 ffffc90004301100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 ================================================================== Tested on: commit: e0dccc3b Linux 5.19-rc8 git tree: https://github.com/torvalds/linux.git console output: https://syzkaller.appspot.com/x/log.txt?x=1156281e080000 kernel config: https://syzkaller.appspot.com/x/.config?x=26034e6fe0075dad dashboard link: https://syzkaller.appspot.com/bug?extid=14b0e8f3fd1612e35350 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 patch: https://syzkaller.appspot.com/x/patch.diff?x=1346ae16080000 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-07-30 8:12 ` Khalid Masum 2022-07-30 10:55 ` syzbot @ 2022-08-01 10:43 ` Dan Carpenter 2022-08-01 14:06 ` Khalid Masum 1 sibling, 1 reply; 28+ messages in thread From: Dan Carpenter @ 2022-08-01 10:43 UTC (permalink / raw) To: Khalid Masum Cc: linux-kernel, syzkaller-bugs, syzbot+14b0e8f3fd1612e35350, khalid.masum On Sat, Jul 30, 2022 at 02:12:46PM +0600, Khalid Masum wrote: > Currently the if block's condition has an unhandled case, where the > result of ret might get greater than vc->vc_scr_end, and therefore > the corresponding handler in else block never gets executed. Which > eventually causes panic in fast_imageblit. > > Add this extra check in the conditions to fix this breakage. > > #syz-test: https://github.com/torvalds/linux.git e0dccc3b76fb > > --- > drivers/video/fbdev/core/fbcon.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > index 1a9aa12cf886..d026f3845b60 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -2591,14 +2591,13 @@ static unsigned long fbcon_getxy(struct vc_data *vc, unsigned long pos, > { > unsigned long ret; > int x, y; > + unsigned long offset = (pos - vc->vc_origin) / 2; > + x = offset % vc->vc_cols; > + y = offset / vc->vc_cols; > + ret = pos + (vc->vc_cols - x) * 2; > > - if (pos >= vc->vc_origin && pos < vc->vc_scr_end) { > - unsigned long offset = (pos - vc->vc_origin) / 2; > - > - x = offset % vc->vc_cols; > - y = offset / vc->vc_cols; > - ret = pos + (vc->vc_cols - x) * 2; > - } else { > + if (!pos >= vc->vc_origin || !pos < vc->vc_scr_end || > + !ret < vc->vc_scr_end) { These are precendence bugs. The ! will be done before the >=. Write it as: if (pos < vc->vc_origin || pos >= vc->vc_scr_end || ret >= vc->vc_scr_end) { > /* Should not happen */ > x = y = 0; > ret = vc->vc_origin; regards, dan carpenter ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-08-01 10:43 ` Dan Carpenter @ 2022-08-01 14:06 ` Khalid Masum 0 siblings, 0 replies; 28+ messages in thread From: Khalid Masum @ 2022-08-01 14:06 UTC (permalink / raw) To: Dan Carpenter Cc: linux-kernel, syzkaller-bugs, syzbot+14b0e8f3fd1612e35350, khalid.masum On 8/1/22 16:43, Dan Carpenter wrote: > > These are precendence bugs. The ! will be done before the >=. Write it > as: > > if (pos < vc->vc_origin || pos >= vc->vc_scr_end || > ret >= vc->vc_scr_end) { > > >> /* Should not happen */ >> x = y = 0; >> ret = vc->vc_origin; > > regards, > dan carpenter > Thanks for the catch. I shall send another syz-test patch with these fixed. thanks, -- Khalid Masum ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2021-11-19 9:18 [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) syzbot ` (3 preceding siblings ...) 2022-07-30 8:12 ` Khalid Masum @ 2022-07-30 11:45 ` Khalid Masum 2022-07-30 15:39 ` syzbot 4 siblings, 1 reply; 28+ messages in thread From: Khalid Masum @ 2022-07-30 11:45 UTC (permalink / raw) To: syzbot+14b0e8f3fd1612e35350; +Cc: linux-kernel, syzkaller-bugs, Khalid Masum #syz-test: https://github.com/torvalds/linux.git e0dccc3b76fb --- drivers/video/fbdev/core/fbcon.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 1a9aa12cf886..d026f3845b60 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -2591,14 +2591,13 @@ static unsigned long fbcon_getxy(struct vc_data *vc, unsigned long pos, { unsigned long ret; int x, y; + unsigned long offset = (pos - vc->vc_origin) / 2; + x = offset % vc->vc_cols; + y = offset / vc->vc_cols; + ret = pos + (vc->vc_cols - x) * 2; - if (pos >= vc->vc_origin && pos < vc->vc_scr_end) { - unsigned long offset = (pos - vc->vc_origin) / 2; - - x = offset % vc->vc_cols; - y = offset / vc->vc_cols; - ret = pos + (vc->vc_cols - x) * 2; - } else { + if (!pos >= vc->vc_origin || !pos < vc->vc_scr_end || + !ret >= vc->vc_origin || !ret < vc->vc_scr_end) { /* Should not happen */ x = y = 0; ret = vc->vc_origin; -- 2.36.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) 2022-07-30 11:45 ` Khalid Masum @ 2022-07-30 15:39 ` syzbot 0 siblings, 0 replies; 28+ messages in thread From: syzbot @ 2022-07-30 15:39 UTC (permalink / raw) To: khalid.masum.92, linux-kernel, syzkaller-bugs Hello, syzbot has tested the proposed patch but the reproducer is still triggering an issue: KASAN: vmalloc-out-of-bounds Write in imageblit ================================================================== BUG: KASAN: vmalloc-out-of-bounds in fast_imageblit drivers/video/fbdev/core/sysimgblt.c:257 [inline] BUG: KASAN: vmalloc-out-of-bounds in sys_imageblit+0x1ed0/0x2240 drivers/video/fbdev/core/sysimgblt.c:323 Write of size 4 at addr ffffc90004411000 by task syz-executor.5/4188 CPU: 1 PID: 4188 Comm: syz-executor.5 Not tainted 5.19.0-rc8-syzkaller-dirty #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/22/2022 Call Trace: <TASK> __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106 print_address_description.constprop.0.cold+0xf/0x495 mm/kasan/report.c:313 print_report mm/kasan/report.c:429 [inline] kasan_report.cold+0xf4/0x1c6 mm/kasan/report.c:491 fast_imageblit drivers/video/fbdev/core/sysimgblt.c:257 [inline] sys_imageblit+0x1ed0/0x2240 drivers/video/fbdev/core/sysimgblt.c:323 drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:825 [inline] drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2328 bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:139 [inline] bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:188 fbcon_putcs+0x314/0x3e0 drivers/video/fbdev/core/fbcon.c:1285 do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 redraw_screen+0x61f/0x740 drivers/tty/vt/vt.c:1035 fbcon_do_set_font+0x5eb/0x6f0 drivers/video/fbdev/core/fbcon.c:2435 fbcon_set_font+0x89d/0xab0 drivers/video/fbdev/core/fbcon.c:2522 con_font_set drivers/tty/vt/vt.c:4666 [inline] con_font_op+0x73a/0xc90 drivers/tty/vt/vt.c:4710 vt_k_ioctl drivers/tty/vt/vt_ioctl.c:474 [inline] vt_ioctl+0x1efa/0x2b20 drivers/tty/vt/vt_ioctl.c:752 tty_ioctl+0xbbd/0x15e0 drivers/tty/tty_io.c:2778 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:870 [inline] __se_sys_ioctl fs/ioctl.c:856 [inline] __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:856 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x7fb2a0689109 Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 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 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007fb2a1830168 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 00007fb2a079bf60 RCX: 00007fb2a0689109 RDX: 0000000020000040 RSI: 0000000000004b72 RDI: 0000000000000004 RBP: 00007fb2a18301d0 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001 R13: 00007ffca595479f R14: 00007fb2a1830300 R15: 0000000000022000 </TASK> The buggy address belongs to the virtual mapping at [ffffc90004111000, ffffc90004412000) created by: drm_gem_shmem_vmap_locked drivers/gpu/drm/drm_gem_shmem_helper.c:319 [inline] drm_gem_shmem_vmap+0x3d7/0x5a0 drivers/gpu/drm/drm_gem_shmem_helper.c:366 Memory state around the buggy address: ffffc90004410f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ffffc90004410f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >ffffc90004411000: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 ^ ffffc90004411080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 ffffc90004411100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 ================================================================== Tested on: commit: e0dccc3b Linux 5.19-rc8 git tree: https://github.com/torvalds/linux.git console output: https://syzkaller.appspot.com/x/log.txt?x=13963ed2080000 kernel config: https://syzkaller.appspot.com/x/.config?x=26034e6fe0075dad dashboard link: https://syzkaller.appspot.com/bug?extid=14b0e8f3fd1612e35350 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 patch: https://syzkaller.appspot.com/x/patch.diff?x=14d7b5da080000 ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2022-08-05 11:38 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-19 9:18 [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) syzbot 2022-01-20 22:58 ` syzbot 2022-01-21 1:48 ` syzbot 2022-07-29 6:51 ` Khalid Masum 2022-07-30 17:25 ` Helge Deller 2022-07-30 18:49 ` [PATCH] tty: vt: selection: Add check for valid tiocl_selection values Helge Deller 2022-08-04 5:47 ` Jiri Slaby 2022-08-04 7:15 ` Helge Deller 2022-08-04 8:44 ` Helge Deller 2022-08-04 9:22 ` Jiri Slaby 2022-08-05 11:13 ` Adam Borowski 2022-08-05 11:13 ` Adam Borowski 2022-07-30 18:50 ` [PATCH] vt: Clear selection before changing the font Helge Deller 2022-07-31 11:32 ` Khalid Masum 2022-07-31 10:03 ` [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) Khalid Masum 2022-07-31 10:54 ` Helge Deller 2022-07-31 10:54 ` Helge Deller 2022-07-31 11:23 ` Khalid Masum 2022-07-31 11:23 ` Khalid Masum 2022-07-31 13:55 ` Khalid Masum 2022-07-31 15:39 ` Helge Deller 2022-08-01 4:09 ` Khalid Masum 2022-07-30 8:12 ` Khalid Masum 2022-07-30 10:55 ` syzbot 2022-08-01 10:43 ` Dan Carpenter 2022-08-01 14:06 ` Khalid Masum 2022-07-30 11:45 ` Khalid Masum 2022-07-30 15:39 ` syzbot
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.