* [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 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
* 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
* [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.