* [syzbot] KMSAN: uninit-value in video_usercopy (2) @ 2021-03-16 10:18 syzbot 2021-03-16 10:44 ` Dmitry Vyukov 2021-06-18 10:34 ` [PATCH] media: v4l2-ioctl: explicitly initialize argument buffer Tetsuo Handa 0 siblings, 2 replies; 9+ messages in thread From: syzbot @ 2021-03-16 10:18 UTC (permalink / raw) To: arnd, glider, hverkuil-cisco, laurent.pinchart, linux-kernel, linux-media, mchehab, niklas.soderlund+renesas, sakari.ailus, sergey.senozhatsky, syzkaller-bugs, yepeilin.cs Hello, syzbot found the following issue on: HEAD commit: 29ad81a1 arch/x86: add missing include to sparsemem.h git tree: https://github.com/google/kmsan.git master console output: https://syzkaller.appspot.com/x/log.txt?x=102502dcd00000 kernel config: https://syzkaller.appspot.com/x/.config?x=8b976581f6bd1e7d dashboard link: https://syzkaller.appspot.com/bug?extid=142888ffec98ab194028 compiler: Debian clang version 11.0.1-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+142888ffec98ab194028@syzkaller.appspotmail.com ===================================================== BUG: KMSAN: uninit-value in check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline] BUG: KMSAN: uninit-value in video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315 CPU: 0 PID: 19595 Comm: syz-executor.4 Not tainted 5.11.0-rc7-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:79 [inline] dump_stack+0x21c/0x280 lib/dump_stack.c:120 kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118 __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197 check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline] video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315 video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391 v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360 v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248 __do_compat_sys_ioctl fs/ioctl.c:842 [inline] __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793 __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793 do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline] __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141 do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166 do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209 entry_SYSENTER_compat_after_hwframe+0x4d/0x5c RIP: 0023:0xf7fec549 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:00000000f55e65fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036 RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c050565d RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 Local variable ----sbuf@video_usercopy created at: video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 ===================================================== ===================================================== BUG: KMSAN: uninit-value in check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 CPU: 0 PID: 19595 Comm: syz-executor.4 Tainted: G B 5.11.0-rc7-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:79 [inline] dump_stack+0x21c/0x280 lib/dump_stack.c:120 kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118 __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197 check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 v4l_prepare_buf+0xbf/0x1d0 drivers/media/v4l2-core/v4l2-ioctl.c:2107 __video_do_ioctl+0x15cd/0x1d20 drivers/media/v4l2-core/v4l2-ioctl.c:2993 video_usercopy+0x2313/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3345 video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391 v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360 v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248 __do_compat_sys_ioctl fs/ioctl.c:842 [inline] __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793 __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793 do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline] __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141 do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166 do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209 entry_SYSENTER_compat_after_hwframe+0x4d/0x5c RIP: 0023:0xf7fec549 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:00000000f55e65fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036 RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c050565d RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 Local variable ----sbuf@video_usercopy created at: video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 ===================================================== --- 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] 9+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in video_usercopy (2) 2021-03-16 10:18 [syzbot] KMSAN: uninit-value in video_usercopy (2) syzbot @ 2021-03-16 10:44 ` Dmitry Vyukov 2021-03-16 13:55 ` Arnd Bergmann 2021-06-18 10:34 ` [PATCH] media: v4l2-ioctl: explicitly initialize argument buffer Tetsuo Handa 1 sibling, 1 reply; 9+ messages in thread From: Dmitry Vyukov @ 2021-03-16 10:44 UTC (permalink / raw) To: syzbot Cc: Arnd Bergmann, Alexander Potapenko, hverkuil-cisco, Laurent Pinchart, LKML, linux-media, Mauro Carvalho Chehab, niklas.soderlund+renesas, sakari.ailus, Sergey Senozhatsky, syzkaller-bugs, Peilin Ye On Tue, Mar 16, 2021 at 11:18 AM syzbot <syzbot+142888ffec98ab194028@syzkaller.appspotmail.com> wrote: > > Hello, > > syzbot found the following issue on: > > HEAD commit: 29ad81a1 arch/x86: add missing include to sparsemem.h > git tree: https://github.com/google/kmsan.git master > console output: https://syzkaller.appspot.com/x/log.txt?x=102502dcd00000 > kernel config: https://syzkaller.appspot.com/x/.config?x=8b976581f6bd1e7d > dashboard link: https://syzkaller.appspot.com/bug?extid=142888ffec98ab194028 > compiler: Debian clang version 11.0.1-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+142888ffec98ab194028@syzkaller.appspotmail.com > > ===================================================== > BUG: KMSAN: uninit-value in check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline] > BUG: KMSAN: uninit-value in video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315 > CPU: 0 PID: 19595 Comm: syz-executor.4 Not tainted 5.11.0-rc7-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:79 [inline] > dump_stack+0x21c/0x280 lib/dump_stack.c:120 > kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118 > __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197 > check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline] > video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315 > video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391 > v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360 > v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248 > __do_compat_sys_ioctl fs/ioctl.c:842 [inline] > __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793 > __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793 > do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline] > __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141 > do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166 > do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209 > entry_SYSENTER_compat_after_hwframe+0x4d/0x5c > RIP: 0023:0xf7fec549 > 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:00000000f55e65fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036 > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c050565d > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 > RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 > > Local variable ----sbuf@video_usercopy created at: > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > ===================================================== > ===================================================== > BUG: KMSAN: uninit-value in check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 > CPU: 0 PID: 19595 Comm: syz-executor.4 Tainted: G B 5.11.0-rc7-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:79 [inline] > dump_stack+0x21c/0x280 lib/dump_stack.c:120 > kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118 > __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197 > check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 > v4l_prepare_buf+0xbf/0x1d0 drivers/media/v4l2-core/v4l2-ioctl.c:2107 > __video_do_ioctl+0x15cd/0x1d20 drivers/media/v4l2-core/v4l2-ioctl.c:2993 > video_usercopy+0x2313/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3345 > video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391 > v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360 > v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248 > __do_compat_sys_ioctl fs/ioctl.c:842 [inline] > __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793 > __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793 > do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline] > __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141 > do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166 > do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209 > entry_SYSENTER_compat_after_hwframe+0x4d/0x5c > RIP: 0023:0xf7fec549 > 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:00000000f55e65fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036 > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c050565d > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 > RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 > > Local variable ----sbuf@video_usercopy created at: > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > ===================================================== I did not get to the very bottom of this, but I looked at this a bit. It seems to be related to some unfortunate interaction of compat syscall and CONFIG_COMPAT_32BIT_TIME. It seems that in this case nothing at all is copied from userspace because cmd gets messed up or something. Perhaps VIDIOC_QUERYBUF is translated into VIDIOC_QUERYBUF_TIME32 instead of VIDIOC_QUERYBUF32_TIME32 and then this gets into compat syscall path and v4l2_compat_get_user does not recognize the command, copies nothing but returns 0. > --- > 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. > > -- > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/0000000000005ace4405bda4af71%40google.com. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in video_usercopy (2) 2021-03-16 10:44 ` Dmitry Vyukov @ 2021-03-16 13:55 ` Arnd Bergmann 2021-03-16 14:02 ` Dmitry Vyukov 2021-03-16 14:05 ` Dmitry Vyukov 0 siblings, 2 replies; 9+ messages in thread From: Arnd Bergmann @ 2021-03-16 13:55 UTC (permalink / raw) To: Dmitry Vyukov Cc: syzbot, Alexander Potapenko, Hans Verkuil, Laurent Pinchart, LKML, Linux Media Mailing List, Mauro Carvalho Chehab, Niklas Söderlund, Sakari Ailus, Sergey Senozhatsky, syzkaller-bugs, Peilin Ye On Tue, Mar 16, 2021 at 11:44 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Tue, Mar 16, 2021 at 11:18 AM syzbot > <syzbot+142888ffec98ab194028@syzkaller.appspotmail.com> wrote: > > > > Hello, > > > > syzbot found the following issue on: > > > > HEAD commit: 29ad81a1 arch/x86: add missing include to sparsemem.h This tree seems to be missing fb18802a338b ("media: v4l: ioctl: Fix memory leak in video_usercopy"), which rewrote that function partly and might fix the problem. > > Local variable ----sbuf@video_usercopy created at: > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > ===================================================== > > ===================================================== > > BUG: KMSAN: uninit-value in check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 > > CPU: 0 PID: 19595 Comm: syz-executor.4 Tainted: G B 5.11.0-rc7-syzkaller #0 > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > > Call Trace: > > __dump_stack lib/dump_stack.c:79 [inline] > > dump_stack+0x21c/0x280 lib/dump_stack.c:120 > > kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118 > > __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197 > > check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 > > v4l_prepare_buf+0xbf/0x1d0 drivers/media/v4l2-core/v4l2-ioctl.c:2107 > > __video_do_ioctl+0x15cd/0x1d20 drivers/media/v4l2-core/v4l2-ioctl.c:2993 > > video_usercopy+0x2313/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3345 > > video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391 > > v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360 > > v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248 > > __do_compat_sys_ioctl fs/ioctl.c:842 [inline] > > __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793 > > __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793 > > do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline] > > __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141 > > do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166 > > do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209 > > entry_SYSENTER_compat_after_hwframe+0x4d/0x5c > > RIP: 0023:0xf7fec549 > > 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:00000000f55e65fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036 > > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c050565d > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 > > RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 > > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 > > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 > > > > Local variable ----sbuf@video_usercopy created at: > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > ===================================================== > > I did not get to the very bottom of this, but I looked at this a bit. > It seems to be related to some unfortunate interaction of compat > syscall and CONFIG_COMPAT_32BIT_TIME. It seems that in this case > nothing at all is copied from userspace because cmd gets messed up or > something. Perhaps VIDIOC_QUERYBUF is translated into > VIDIOC_QUERYBUF_TIME32 instead of VIDIOC_QUERYBUF32_TIME32 and then > this gets into compat syscall path and v4l2_compat_get_user does not > recognize the command, copies nothing but returns 0. User space would be calling VIDIOC_QUERYBUF32_TIME32 here, if it's built against glibc, though with a musl based user space, you would get called with VIDIOC_QUERYBUF32. What I notice in get_v4l2_buffer32_time32(), is that we do a full copy_from_user() to the stack of this function, and then copy the members individually to the output v4l2_buffer structure: struct v4l2_buffer32_time32 vb32; if (copy_from_user(&vb32, arg, sizeof(vb32))) return -EFAULT; *vb = (struct v4l2_buffer) { .index = vb32.index, .type = vb32.type, .bytesused = vb32.bytesused, .flags = vb32.flags, .field = vb32.field, .timestamp.tv_sec = vb32.timestamp.tv_sec, .timestamp.tv_usec = vb32.timestamp.tv_usec, .timecode = vb32.timecode, .sequence = vb32.sequence, .memory = vb32.memory, .m.offset = vb32.m.offset, .length = vb32.length, .request_fd = vb32.request_fd, }; This struct assignment will however leave any padding fields uninitialized. There is padding between 'field' and 'timestamp. Could that trigger a KMSAN bug? Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in video_usercopy (2) 2021-03-16 13:55 ` Arnd Bergmann @ 2021-03-16 14:02 ` Dmitry Vyukov 2021-03-16 15:06 ` Arnd Bergmann 2021-03-16 14:05 ` Dmitry Vyukov 1 sibling, 1 reply; 9+ messages in thread From: Dmitry Vyukov @ 2021-03-16 14:02 UTC (permalink / raw) To: Arnd Bergmann Cc: syzbot, Alexander Potapenko, Hans Verkuil, Laurent Pinchart, LKML, Linux Media Mailing List, Mauro Carvalho Chehab, Niklas Söderlund, Sakari Ailus, Sergey Senozhatsky, syzkaller-bugs, Peilin Ye On Tue, Mar 16, 2021 at 2:56 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Mar 16, 2021 at 11:44 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > On Tue, Mar 16, 2021 at 11:18 AM syzbot > > <syzbot+142888ffec98ab194028@syzkaller.appspotmail.com> wrote: > > > > > > Hello, > > > > > > syzbot found the following issue on: > > > > > > HEAD commit: 29ad81a1 arch/x86: add missing include to sparsemem.h > > This tree seems to be missing fb18802a338b ("media: v4l: ioctl: Fix memory > leak in video_usercopy"), which rewrote that function partly and might > fix the problem. > > > > Local variable ----sbuf@video_usercopy created at: > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > ===================================================== > > > ===================================================== > > > BUG: KMSAN: uninit-value in check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 > > > CPU: 0 PID: 19595 Comm: syz-executor.4 Tainted: G B 5.11.0-rc7-syzkaller #0 > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > > > Call Trace: > > > __dump_stack lib/dump_stack.c:79 [inline] > > > dump_stack+0x21c/0x280 lib/dump_stack.c:120 > > > kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118 > > > __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197 > > > check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 > > > v4l_prepare_buf+0xbf/0x1d0 drivers/media/v4l2-core/v4l2-ioctl.c:2107 > > > __video_do_ioctl+0x15cd/0x1d20 drivers/media/v4l2-core/v4l2-ioctl.c:2993 > > > video_usercopy+0x2313/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3345 > > > video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391 > > > v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360 > > > v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248 > > > __do_compat_sys_ioctl fs/ioctl.c:842 [inline] > > > __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793 > > > __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793 > > > do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline] > > > __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141 > > > do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166 > > > do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209 > > > entry_SYSENTER_compat_after_hwframe+0x4d/0x5c > > > RIP: 0023:0xf7fec549 > > > 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:00000000f55e65fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036 > > > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c050565d > > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 > > > RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 > > > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 > > > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 > > > > > > Local variable ----sbuf@video_usercopy created at: > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > ===================================================== > > > > I did not get to the very bottom of this, but I looked at this a bit. > > It seems to be related to some unfortunate interaction of compat > > syscall and CONFIG_COMPAT_32BIT_TIME. It seems that in this case > > nothing at all is copied from userspace because cmd gets messed up or > > something. Perhaps VIDIOC_QUERYBUF is translated into > > VIDIOC_QUERYBUF_TIME32 instead of VIDIOC_QUERYBUF32_TIME32 and then > > this gets into compat syscall path and v4l2_compat_get_user does not > > recognize the command, copies nothing but returns 0. > > User space would be calling VIDIOC_QUERYBUF32_TIME32 here, > if it's built against glibc, though with a musl based user space, you > would get called with VIDIOC_QUERYBUF32. Or somebody fetching somebody else's credit card number will be calling VIDIOC_QUERYBUF_TIME32 directly ;) > What I notice in get_v4l2_buffer32_time32(), is that we do a full > copy_from_user() to the stack of this function, and then copy the > members individually to the output v4l2_buffer structure: > > struct v4l2_buffer32_time32 vb32; > if (copy_from_user(&vb32, arg, sizeof(vb32))) > return -EFAULT; > *vb = (struct v4l2_buffer) { > .index = vb32.index, > .type = vb32.type, > .bytesused = vb32.bytesused, > .flags = vb32.flags, > .field = vb32.field, > .timestamp.tv_sec = vb32.timestamp.tv_sec, > .timestamp.tv_usec = vb32.timestamp.tv_usec, > .timecode = vb32.timecode, > .sequence = vb32.sequence, > .memory = vb32.memory, > .m.offset = vb32.m.offset, > .length = vb32.length, > .request_fd = vb32.request_fd, > }; > > This struct assignment will however leave any padding > fields uninitialized. There is padding between 'field' and > 'timestamp. Could that trigger a KMSAN bug? Report seems to be saying it's vb.type that's uninitialized. I suspect we copy nothing at all from user space. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in video_usercopy (2) 2021-03-16 14:02 ` Dmitry Vyukov @ 2021-03-16 15:06 ` Arnd Bergmann 2021-03-16 17:26 ` Hans Verkuil 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2021-03-16 15:06 UTC (permalink / raw) To: Dmitry Vyukov Cc: syzbot, Alexander Potapenko, Hans Verkuil, Laurent Pinchart, LKML, Linux Media Mailing List, Mauro Carvalho Chehab, Niklas Söderlund, Sakari Ailus, Sergey Senozhatsky, syzkaller-bugs, Peilin Ye On Tue, Mar 16, 2021 at 3:02 PM Dmitry Vyukov <dvyukov@google.com> wrote: > On Tue, Mar 16, 2021 at 2:56 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Mar 16, 2021 at 11:44 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > On Tue, Mar 16, 2021 at 11:18 AM syzbot > > > <syzbot+142888ffec98ab194028@syzkaller.appspotmail.com> wrote: > > > > > > > > Hello, > > > > > > > > syzbot found the following issue on: > > > > > > > > HEAD commit: 29ad81a1 arch/x86: add missing include to sparsemem.h > > > > This tree seems to be missing fb18802a338b ("media: v4l: ioctl: Fix memory > > leak in video_usercopy"), which rewrote that function partly and might > > fix the problem. > > > > > > Local variable ----sbuf@video_usercopy created at: > > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > > ===================================================== > > > > ===================================================== > > > > BUG: KMSAN: uninit-value in check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 > > > > CPU: 0 PID: 19595 Comm: syz-executor.4 Tainted: G B 5.11.0-rc7-syzkaller #0 > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > > > > Call Trace: > > > > __dump_stack lib/dump_stack.c:79 [inline] > > > > dump_stack+0x21c/0x280 lib/dump_stack.c:120 > > > > kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118 > > > > __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197 > > > > check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 > > > > v4l_prepare_buf+0xbf/0x1d0 drivers/media/v4l2-core/v4l2-ioctl.c:2107 > > > > __video_do_ioctl+0x15cd/0x1d20 drivers/media/v4l2-core/v4l2-ioctl.c:2993 > > > > video_usercopy+0x2313/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3345 > > > > video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391 > > > > v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360 > > > > v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248 > > > > __do_compat_sys_ioctl fs/ioctl.c:842 [inline] > > > > __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793 > > > > __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793 > > > > do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline] > > > > __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141 > > > > do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166 > > > > do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209 > > > > entry_SYSENTER_compat_after_hwframe+0x4d/0x5c > > > > RIP: 0023:0xf7fec549 > > > > 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:00000000f55e65fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036 > > > > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c050565d > > > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 > > > > RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 > > > > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 > > > > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 > > > > > > > > Local variable ----sbuf@video_usercopy created at: > > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > > ===================================================== > > > > > > I did not get to the very bottom of this, but I looked at this a bit. > > > It seems to be related to some unfortunate interaction of compat > > > syscall and CONFIG_COMPAT_32BIT_TIME. It seems that in this case > > > nothing at all is copied from userspace because cmd gets messed up or > > > something. Perhaps VIDIOC_QUERYBUF is translated into > > > VIDIOC_QUERYBUF_TIME32 instead of VIDIOC_QUERYBUF32_TIME32 and then > > > this gets into compat syscall path and v4l2_compat_get_user does not > > > recognize the command, copies nothing but returns 0. > > > > User space would be calling VIDIOC_QUERYBUF32_TIME32 here, > > if it's built against glibc, though with a musl based user space, you > > would get called with VIDIOC_QUERYBUF32. > > Or somebody fetching somebody else's credit card number will be > calling VIDIOC_QUERYBUF_TIME32 directly ;) Ah of course, I forgot the ioctl command may already be fuzzed here. When I look at https://syzkaller.appspot.com/text?tag=CrashLog&x=12bd0e3ad00000 I see 0xc0585609, which would be a VIDIOC_QUERYBUF with size=0x58, which is the native ioctl, not the compat one. This is something we didn't expect to get passed into the compat ioctl handler, but should of course handle gracefully If the command were to get is the 64-bit version of VIDIOC_QUERYBUF_TIME32 (0xc0505609), then it gets converted to VIDIOC_QUERYBUF by video_translate_cmd(). If it's VIDIOC_QUERYBUF, it stays that way. It does break down in v4l2_compat_get_user() when we get called with VIDIOC_QUERYBUF_TIME32, since that leads to not copying at all, as you guessed. I think this should fix the case of passing VIDIOC_QUERYBUF_TIME32: --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -3115,7 +3115,7 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size, static unsigned int video_translate_cmd(unsigned int cmd) { switch (cmd) { -#ifdef CONFIG_COMPAT_32BIT_TIME +#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME) case VIDIOC_DQEVENT_TIME32: return VIDIOC_DQEVENT; case VIDIOC_QUERYBUF_TIME32: Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in video_usercopy (2) 2021-03-16 15:06 ` Arnd Bergmann @ 2021-03-16 17:26 ` Hans Verkuil 0 siblings, 0 replies; 9+ messages in thread From: Hans Verkuil @ 2021-03-16 17:26 UTC (permalink / raw) To: Arnd Bergmann, Dmitry Vyukov Cc: syzbot, Alexander Potapenko, Laurent Pinchart, LKML, Linux Media Mailing List, Mauro Carvalho Chehab, Niklas Söderlund, Sakari Ailus, Sergey Senozhatsky, syzkaller-bugs, Peilin Ye On 16/03/2021 16:06, Arnd Bergmann wrote: > On Tue, Mar 16, 2021 at 3:02 PM Dmitry Vyukov <dvyukov@google.com> wrote: >> On Tue, Mar 16, 2021 at 2:56 PM Arnd Bergmann <arnd@arndb.de> wrote: >>> On Tue, Mar 16, 2021 at 11:44 AM Dmitry Vyukov <dvyukov@google.com> wrote: >>>> On Tue, Mar 16, 2021 at 11:18 AM syzbot >>>> <syzbot+142888ffec98ab194028@syzkaller.appspotmail.com> wrote: >>>>> >>>>> Hello, >>>>> >>>>> syzbot found the following issue on: >>>>> >>>>> HEAD commit: 29ad81a1 arch/x86: add missing include to sparsemem.h >>> >>> This tree seems to be missing fb18802a338b ("media: v4l: ioctl: Fix memory >>> leak in video_usercopy"), which rewrote that function partly and might >>> fix the problem. >>> >>>>> Local variable ----sbuf@video_usercopy created at: >>>>> video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 >>>>> video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 >>>>> ===================================================== >>>>> ===================================================== >>>>> BUG: KMSAN: uninit-value in check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 >>>>> CPU: 0 PID: 19595 Comm: syz-executor.4 Tainted: G B 5.11.0-rc7-syzkaller #0 >>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 >>>>> Call Trace: >>>>> __dump_stack lib/dump_stack.c:79 [inline] >>>>> dump_stack+0x21c/0x280 lib/dump_stack.c:120 >>>>> kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118 >>>>> __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197 >>>>> check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 >>>>> v4l_prepare_buf+0xbf/0x1d0 drivers/media/v4l2-core/v4l2-ioctl.c:2107 >>>>> __video_do_ioctl+0x15cd/0x1d20 drivers/media/v4l2-core/v4l2-ioctl.c:2993 >>>>> video_usercopy+0x2313/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3345 >>>>> video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391 >>>>> v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360 >>>>> v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248 >>>>> __do_compat_sys_ioctl fs/ioctl.c:842 [inline] >>>>> __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793 >>>>> __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793 >>>>> do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline] >>>>> __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141 >>>>> do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166 >>>>> do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209 >>>>> entry_SYSENTER_compat_after_hwframe+0x4d/0x5c >>>>> RIP: 0023:0xf7fec549 >>>>> 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:00000000f55e65fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036 >>>>> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c050565d >>>>> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 >>>>> RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 >>>>> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 >>>>> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 >>>>> >>>>> Local variable ----sbuf@video_usercopy created at: >>>>> video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 >>>>> video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 >>>>> ===================================================== >>>> >>>> I did not get to the very bottom of this, but I looked at this a bit. >>>> It seems to be related to some unfortunate interaction of compat >>>> syscall and CONFIG_COMPAT_32BIT_TIME. It seems that in this case >>>> nothing at all is copied from userspace because cmd gets messed up or >>>> something. Perhaps VIDIOC_QUERYBUF is translated into >>>> VIDIOC_QUERYBUF_TIME32 instead of VIDIOC_QUERYBUF32_TIME32 and then >>>> this gets into compat syscall path and v4l2_compat_get_user does not >>>> recognize the command, copies nothing but returns 0. >>> >>> User space would be calling VIDIOC_QUERYBUF32_TIME32 here, >>> if it's built against glibc, though with a musl based user space, you >>> would get called with VIDIOC_QUERYBUF32. >> >> Or somebody fetching somebody else's credit card number will be >> calling VIDIOC_QUERYBUF_TIME32 directly ;) > > Ah of course, I forgot the ioctl command may already be fuzzed here. > > When I look at > https://syzkaller.appspot.com/text?tag=CrashLog&x=12bd0e3ad00000 > > I see 0xc0585609, which would be a VIDIOC_QUERYBUF with > size=0x58, which is the native ioctl, not the compat one. This > is something we didn't expect to get passed into the compat ioctl > handler, but should of course handle gracefully > > If the command were to get is the 64-bit version of > VIDIOC_QUERYBUF_TIME32 (0xc0505609), then it gets converted to > VIDIOC_QUERYBUF by video_translate_cmd(). > If it's VIDIOC_QUERYBUF, it stays that way. > > It does break down in v4l2_compat_get_user() when we get > called with VIDIOC_QUERYBUF_TIME32, since that leads > to not copying at all, as you guessed. > > I think this should fix the case of passing > VIDIOC_QUERYBUF_TIME32: I tested this and I can confirm that this works. Arnd, do you want to make a patch for this? If so, you can add my Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Regards, Hans > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -3115,7 +3115,7 @@ static int check_array_args(unsigned int cmd, > void *parg, size_t *array_size, > static unsigned int video_translate_cmd(unsigned int cmd) > { > switch (cmd) { > -#ifdef CONFIG_COMPAT_32BIT_TIME > +#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME) > case VIDIOC_DQEVENT_TIME32: > return VIDIOC_DQEVENT; > case VIDIOC_QUERYBUF_TIME32: > > Arnd > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in video_usercopy (2) 2021-03-16 13:55 ` Arnd Bergmann 2021-03-16 14:02 ` Dmitry Vyukov @ 2021-03-16 14:05 ` Dmitry Vyukov 1 sibling, 0 replies; 9+ messages in thread From: Dmitry Vyukov @ 2021-03-16 14:05 UTC (permalink / raw) To: Arnd Bergmann Cc: syzbot, Alexander Potapenko, Hans Verkuil, Laurent Pinchart, LKML, Linux Media Mailing List, Mauro Carvalho Chehab, Niklas Söderlund, Sakari Ailus, Sergey Senozhatsky, syzkaller-bugs, Peilin Ye On Tue, Mar 16, 2021 at 2:56 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Mar 16, 2021 at 11:44 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > On Tue, Mar 16, 2021 at 11:18 AM syzbot > > <syzbot+142888ffec98ab194028@syzkaller.appspotmail.com> wrote: > > > > > > Hello, > > > > > > syzbot found the following issue on: > > > > > > HEAD commit: 29ad81a1 arch/x86: add missing include to sparsemem.h > > This tree seems to be missing fb18802a338b ("media: v4l: ioctl: Fix memory > leak in video_usercopy"), which rewrote that function partly and might > fix the problem. As far as I see the bug happens before any of the code changed by fb18802a338b is invoked. > > > Local variable ----sbuf@video_usercopy created at: > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > ===================================================== > > > ===================================================== > > > BUG: KMSAN: uninit-value in check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 > > > CPU: 0 PID: 19595 Comm: syz-executor.4 Tainted: G B 5.11.0-rc7-syzkaller #0 > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > > > Call Trace: > > > __dump_stack lib/dump_stack.c:79 [inline] > > > dump_stack+0x21c/0x280 lib/dump_stack.c:120 > > > kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118 > > > __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197 > > > check_fmt+0x864/0x1070 drivers/media/v4l2-core/v4l2-ioctl.c:963 > > > v4l_prepare_buf+0xbf/0x1d0 drivers/media/v4l2-core/v4l2-ioctl.c:2107 > > > __video_do_ioctl+0x15cd/0x1d20 drivers/media/v4l2-core/v4l2-ioctl.c:2993 > > > video_usercopy+0x2313/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3345 > > > video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391 > > > v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360 > > > v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248 > > > __do_compat_sys_ioctl fs/ioctl.c:842 [inline] > > > __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793 > > > __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793 > > > do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline] > > > __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141 > > > do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166 > > > do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209 > > > entry_SYSENTER_compat_after_hwframe+0x4d/0x5c > > > RIP: 0023:0xf7fec549 > > > 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:00000000f55e65fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036 > > > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c050565d > > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 > > > RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 > > > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 > > > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 > > > > > > Local variable ----sbuf@video_usercopy created at: > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > video_usercopy+0xaa/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3285 > > > ===================================================== > > > > I did not get to the very bottom of this, but I looked at this a bit. > > It seems to be related to some unfortunate interaction of compat > > syscall and CONFIG_COMPAT_32BIT_TIME. It seems that in this case > > nothing at all is copied from userspace because cmd gets messed up or > > something. Perhaps VIDIOC_QUERYBUF is translated into > > VIDIOC_QUERYBUF_TIME32 instead of VIDIOC_QUERYBUF32_TIME32 and then > > this gets into compat syscall path and v4l2_compat_get_user does not > > recognize the command, copies nothing but returns 0. > > User space would be calling VIDIOC_QUERYBUF32_TIME32 here, > if it's built against glibc, though with a musl based user space, you > would get called with VIDIOC_QUERYBUF32. > > What I notice in get_v4l2_buffer32_time32(), is that we do a full > copy_from_user() to the stack of this function, and then copy the > members individually to the output v4l2_buffer structure: > > struct v4l2_buffer32_time32 vb32; > if (copy_from_user(&vb32, arg, sizeof(vb32))) > return -EFAULT; > *vb = (struct v4l2_buffer) { > .index = vb32.index, > .type = vb32.type, > .bytesused = vb32.bytesused, > .flags = vb32.flags, > .field = vb32.field, > .timestamp.tv_sec = vb32.timestamp.tv_sec, > .timestamp.tv_usec = vb32.timestamp.tv_usec, > .timecode = vb32.timecode, > .sequence = vb32.sequence, > .memory = vb32.memory, > .m.offset = vb32.m.offset, > .length = vb32.length, > .request_fd = vb32.request_fd, > }; > > This struct assignment will however leave any padding > fields uninitialized. There is padding between 'field' and > 'timestamp. Could that trigger a KMSAN bug? > > Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] media: v4l2-ioctl: explicitly initialize argument buffer 2021-03-16 10:18 [syzbot] KMSAN: uninit-value in video_usercopy (2) syzbot 2021-03-16 10:44 ` Dmitry Vyukov @ 2021-06-18 10:34 ` Tetsuo Handa 2021-06-18 10:41 ` Arnd Bergmann 1 sibling, 1 reply; 9+ messages in thread From: Tetsuo Handa @ 2021-06-18 10:34 UTC (permalink / raw) To: mchehab, linux-media Cc: syzbot, arnd, glider, hverkuil-cisco, laurent.pinchart, niklas.soderlund+renesas, sakari.ailus, sergey.senozhatsky, syzkaller-bugs, yepeilin.cs KMSAN complains that ioctl(VIDIOC_QUERYBUF_TIME32) copies uninitialized kernel stack memory to userspace [1], for video_usercopy() calls copy_to_user() even if __video_do_ioctl() returned -EINVAL error. Generally, copy_to_user() needn't be called when there was an error. But video_usercopy() has always_copy logic which forces copy_to_user(). Therefore, instead of not calling copy_to_user(), explicitly initialize argument buffer. ---------- /* Compile for 32bit userspace and run on 64bit kernel. */ #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> #include <sys/ioctl.h> #define VIDIOC_QUERYBUF_TIME32 0xc0505609 int main(int argc, char *argv[]) { char buf[128] = { }; ioctl(open("/dev/video0", O_RDONLY), VIDIOC_QUERYBUF_TIME32, &buf); return 0; } ---------- Link: https://syzkaller.appspot.com/bug?id=eb945b02a7b3060a8a60dab673c02f3ab20a048b [1] Reported-by: syzbot <syzbot+142888ffec98ab194028@syzkaller.appspotmail.com> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- drivers/media/v4l2-core/v4l2-ioctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 2673f51aafa4..ba204e0200d3 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -3240,7 +3240,7 @@ long video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg, v4l2_kioctl func) { - char sbuf[128]; + char sbuf[128] = { }; void *mbuf = NULL, *array_buf = NULL; void *parg = (void *)arg; long err = -EINVAL; @@ -3258,7 +3258,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg, parg = sbuf; } else { /* too big to allocate from stack */ - mbuf = kmalloc(ioc_size, GFP_KERNEL); + mbuf = kzalloc(ioc_size, GFP_KERNEL); if (NULL == mbuf) return -ENOMEM; parg = mbuf; -- 2.18.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] media: v4l2-ioctl: explicitly initialize argument buffer 2021-06-18 10:34 ` [PATCH] media: v4l2-ioctl: explicitly initialize argument buffer Tetsuo Handa @ 2021-06-18 10:41 ` Arnd Bergmann 0 siblings, 0 replies; 9+ messages in thread From: Arnd Bergmann @ 2021-06-18 10:41 UTC (permalink / raw) To: Tetsuo Handa Cc: Mauro Carvalho Chehab, Linux Media Mailing List, syzbot, Alexander Potapenko, Hans Verkuil, Laurent Pinchart, Niklas Söderlund, Sakari Ailus, Sergey Senozhatsky, syzkaller-bugs, Peilin Ye On Fri, Jun 18, 2021 at 12:34 PM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > > KMSAN complains that ioctl(VIDIOC_QUERYBUF_TIME32) copies uninitialized > kernel stack memory to userspace [1], for video_usercopy() calls > copy_to_user() even if __video_do_ioctl() returned -EINVAL error. > > Generally, copy_to_user() needn't be called when there was an error. > But video_usercopy() has always_copy logic which forces copy_to_user(). > Therefore, instead of not calling copy_to_user(), explicitly initialize > argument buffer. > > ---------- > /* Compile for 32bit userspace and run on 64bit kernel. */ > #include <sys/types.h> > #include <sys/stat.h> > #include <fcntl.h> > #include <sys/ioctl.h> > #define VIDIOC_QUERYBUF_TIME32 0xc0505609 > > int main(int argc, char *argv[]) > { > char buf[128] = { }; > > ioctl(open("/dev/video0", O_RDONLY), VIDIOC_QUERYBUF_TIME32, &buf); > return 0; > } > ---------- > > Link: https://syzkaller.appspot.com/bug?id=eb945b02a7b3060a8a60dab673c02f3ab20a048b [1] > Reported-by: syzbot <syzbot+142888ffec98ab194028@syzkaller.appspotmail.com> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> This should no longer be necessary once my recent patches propagate into linux-next, see https://patchwork.linuxtv.org/project/linux-media/list/?series=5678&state=* Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-06-18 10:44 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-16 10:18 [syzbot] KMSAN: uninit-value in video_usercopy (2) syzbot 2021-03-16 10:44 ` Dmitry Vyukov 2021-03-16 13:55 ` Arnd Bergmann 2021-03-16 14:02 ` Dmitry Vyukov 2021-03-16 15:06 ` Arnd Bergmann 2021-03-16 17:26 ` Hans Verkuil 2021-03-16 14:05 ` Dmitry Vyukov 2021-06-18 10:34 ` [PATCH] media: v4l2-ioctl: explicitly initialize argument buffer Tetsuo Handa 2021-06-18 10:41 ` Arnd Bergmann
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.