Hello,
syzbot found the following crash on:
HEAD commit: 8bbbc5cf kmsan: don't compile memmove
git tree: https://github.com/google/kmsan.git master
console output: https://syzkaller.appspot.com/x/log.txt?x=11514265e00000
kernel config: https://syzkaller.appspot.com/x/.config?x=cd0e9a6b0e555cc3
dashboard link: https://syzkaller.appspot.com/bug?extid=af962bf9e7e27bccd025
compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
userspace arch: i386
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16a89109e00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=176f774ee00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: [email protected]
=====================================================
BUG: KMSAN: uninit-value in kmsan_check_memory+0xd/0x10 mm/kmsan/kmsan_hooks.c:413
CPU: 1 PID: 11659 Comm: syz-executor923 Not tainted 5.6.0-rc2-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x220 lib/dump_stack.c:118
kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:118
kmsan_internal_check_memory+0x358/0x3d0 mm/kmsan/kmsan.c:457
kmsan_check_memory+0xd/0x10 mm/kmsan/kmsan_hooks.c:413
snapshot_compat_ioctl+0x559/0x650 kernel/power/user.c:422
__do_compat_sys_ioctl fs/ioctl.c:857 [inline]
__se_compat_sys_ioctl+0x57c/0xed0 fs/ioctl.c:808
__ia32_compat_sys_ioctl+0xd9/0x110 fs/ioctl.c:808
do_syscall_32_irqs_on arch/x86/entry/common.c:339 [inline]
do_fast_syscall_32+0x3c7/0x6e0 arch/x86/entry/common.c:410
entry_SYSENTER_compat+0x68/0x77 arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7f70d99
Code: 90 e8 0b 00 00 00 f3 90 0f ae e8 eb f9 8d 74 26 00 89 3c 24 c3 90 90 90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 002b:00000000ffec145c EFLAGS: 00000213 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000080083313
RDX: 0000000000000000 RSI: 00000000080ea078 RDI: 00000000ffec14b0
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
Uninit was stored to memory at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:144 [inline]
kmsan_internal_chain_origin+0xad/0x130 mm/kmsan/kmsan.c:310
__msan_chain_origin+0x50/0x90 mm/kmsan/kmsan_instr.c:165
snapshot_compat_ioctl+0x5e0/0x650 kernel/power/user.c:422
__do_compat_sys_ioctl fs/ioctl.c:857 [inline]
__se_compat_sys_ioctl+0x57c/0xed0 fs/ioctl.c:808
__ia32_compat_sys_ioctl+0xd9/0x110 fs/ioctl.c:808
do_syscall_32_irqs_on arch/x86/entry/common.c:339 [inline]
do_fast_syscall_32+0x3c7/0x6e0 arch/x86/entry/common.c:410
entry_SYSENTER_compat+0x68/0x77 arch/x86/entry/entry_64_compat.S:139
Local variable ----offset@snapshot_compat_ioctl created at:
get_current arch/x86/include/asm/current.h:15 [inline]
snapshot_compat_ioctl+0x324/0x650 kernel/power/user.c:418
get_current arch/x86/include/asm/current.h:15 [inline]
snapshot_compat_ioctl+0x324/0x650 kernel/power/user.c:418
Bytes 0-7 of 8 are uninitialized
Memory access of size 8 starts at ffff9946c156bd30
=====================================================
---
This bug 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 [email protected].
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
On Wed, Feb 26, 2020 at 07:59:13AM -0800, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 8bbbc5cf kmsan: don't compile memmove
> git tree: https://github.com/google/kmsan.git master
> console output: https://syzkaller.appspot.com/x/log.txt?x=11514265e00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=cd0e9a6b0e555cc3
> dashboard link: https://syzkaller.appspot.com/bug?extid=af962bf9e7e27bccd025
> compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
> userspace arch: i386
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16a89109e00000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=176f774ee00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: [email protected]
>
> =====================================================
> BUG: KMSAN: uninit-value in kmsan_check_memory+0xd/0x10 mm/kmsan/kmsan_hooks.c:413
> CPU: 1 PID: 11659 Comm: syz-executor923 Not tainted 5.6.0-rc2-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1c9/0x220 lib/dump_stack.c:118
> kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:118
> kmsan_internal_check_memory+0x358/0x3d0 mm/kmsan/kmsan.c:457
> kmsan_check_memory+0xd/0x10 mm/kmsan/kmsan_hooks.c:413
> snapshot_compat_ioctl+0x559/0x650 kernel/power/user.c:422
> __do_compat_sys_ioctl fs/ioctl.c:857 [inline]
> __se_compat_sys_ioctl+0x57c/0xed0 fs/ioctl.c:808
> __ia32_compat_sys_ioctl+0xd9/0x110 fs/ioctl.c:808
> do_syscall_32_irqs_on arch/x86/entry/common.c:339 [inline]
> do_fast_syscall_32+0x3c7/0x6e0 arch/x86/entry/common.c:410
> entry_SYSENTER_compat+0x68/0x77 arch/x86/entry/entry_64_compat.S:139
> RIP: 0023:0xf7f70d99
> Code: 90 e8 0b 00 00 00 f3 90 0f ae e8 eb f9 8d 74 26 00 89 3c 24 c3 90 90 90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
> RSP: 002b:00000000ffec145c EFLAGS: 00000213 ORIG_RAX: 0000000000000036
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000080083313
> RDX: 0000000000000000 RSI: 00000000080ea078 RDI: 00000000ffec14b0
> RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
>
> Uninit was stored to memory at:
> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:144 [inline]
> kmsan_internal_chain_origin+0xad/0x130 mm/kmsan/kmsan.c:310
> __msan_chain_origin+0x50/0x90 mm/kmsan/kmsan_instr.c:165
> snapshot_compat_ioctl+0x5e0/0x650 kernel/power/user.c:422
> __do_compat_sys_ioctl fs/ioctl.c:857 [inline]
> __se_compat_sys_ioctl+0x57c/0xed0 fs/ioctl.c:808
> __ia32_compat_sys_ioctl+0xd9/0x110 fs/ioctl.c:808
> do_syscall_32_irqs_on arch/x86/entry/common.c:339 [inline]
> do_fast_syscall_32+0x3c7/0x6e0 arch/x86/entry/common.c:410
> entry_SYSENTER_compat+0x68/0x77 arch/x86/entry/entry_64_compat.S:139
>
> Local variable ----offset@snapshot_compat_ioctl created at:
> get_current arch/x86/include/asm/current.h:15 [inline]
> snapshot_compat_ioctl+0x324/0x650 kernel/power/user.c:418
> get_current arch/x86/include/asm/current.h:15 [inline]
> snapshot_compat_ioctl+0x324/0x650 kernel/power/user.c:418
>
> Bytes 0-7 of 8 are uninitialized
> Memory access of size 8 starts at ffff9946c156bd30
> =====================================================
Looks like a KMSAN false positive? As far as I can tell, the memory is being
initialized by put_user() called under set_fs(KERNEL_DS).
On Sat, Mar 07, 2020 at 03:54:37PM -0800, Eric Biggers wrote:
> On Wed, Feb 26, 2020 at 07:59:13AM -0800, syzbot wrote:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: 8bbbc5cf kmsan: don't compile memmove
> > git tree: https://github.com/google/kmsan.git master
> > console output: https://syzkaller.appspot.com/x/log.txt?x=11514265e00000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=cd0e9a6b0e555cc3
> > dashboard link: https://syzkaller.appspot.com/bug?extid=af962bf9e7e27bccd025
> > compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
> > userspace arch: i386
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16a89109e00000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=176f774ee00000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: [email protected]
> >
> > =====================================================
> > BUG: KMSAN: uninit-value in kmsan_check_memory+0xd/0x10 mm/kmsan/kmsan_hooks.c:413
> > CPU: 1 PID: 11659 Comm: syz-executor923 Not tainted 5.6.0-rc2-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > Call Trace:
> > __dump_stack lib/dump_stack.c:77 [inline]
> > dump_stack+0x1c9/0x220 lib/dump_stack.c:118
> > kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:118
> > kmsan_internal_check_memory+0x358/0x3d0 mm/kmsan/kmsan.c:457
> > kmsan_check_memory+0xd/0x10 mm/kmsan/kmsan_hooks.c:413
> > snapshot_compat_ioctl+0x559/0x650 kernel/power/user.c:422
> > __do_compat_sys_ioctl fs/ioctl.c:857 [inline]
> > __se_compat_sys_ioctl+0x57c/0xed0 fs/ioctl.c:808
> > __ia32_compat_sys_ioctl+0xd9/0x110 fs/ioctl.c:808
> > do_syscall_32_irqs_on arch/x86/entry/common.c:339 [inline]
> > do_fast_syscall_32+0x3c7/0x6e0 arch/x86/entry/common.c:410
> > entry_SYSENTER_compat+0x68/0x77 arch/x86/entry/entry_64_compat.S:139
> > RIP: 0023:0xf7f70d99
> > Code: 90 e8 0b 00 00 00 f3 90 0f ae e8 eb f9 8d 74 26 00 89 3c 24 c3 90 90 90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
> > RSP: 002b:00000000ffec145c EFLAGS: 00000213 ORIG_RAX: 0000000000000036
> > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000080083313
> > RDX: 0000000000000000 RSI: 00000000080ea078 RDI: 00000000ffec14b0
> > RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> >
> > Uninit was stored to memory at:
> > kmsan_save_stack_with_flags mm/kmsan/kmsan.c:144 [inline]
> > kmsan_internal_chain_origin+0xad/0x130 mm/kmsan/kmsan.c:310
> > __msan_chain_origin+0x50/0x90 mm/kmsan/kmsan_instr.c:165
> > snapshot_compat_ioctl+0x5e0/0x650 kernel/power/user.c:422
> > __do_compat_sys_ioctl fs/ioctl.c:857 [inline]
> > __se_compat_sys_ioctl+0x57c/0xed0 fs/ioctl.c:808
> > __ia32_compat_sys_ioctl+0xd9/0x110 fs/ioctl.c:808
> > do_syscall_32_irqs_on arch/x86/entry/common.c:339 [inline]
> > do_fast_syscall_32+0x3c7/0x6e0 arch/x86/entry/common.c:410
> > entry_SYSENTER_compat+0x68/0x77 arch/x86/entry/entry_64_compat.S:139
> >
> > Local variable ----offset@snapshot_compat_ioctl created at:
> > get_current arch/x86/include/asm/current.h:15 [inline]
> > snapshot_compat_ioctl+0x324/0x650 kernel/power/user.c:418
> > get_current arch/x86/include/asm/current.h:15 [inline]
> > snapshot_compat_ioctl+0x324/0x650 kernel/power/user.c:418
> >
> > Bytes 0-7 of 8 are uninitialized
> > Memory access of size 8 starts at ffff9946c156bd30
> > =====================================================
>
> Looks like a KMSAN false positive? As far as I can tell, the memory is being
> initialized by put_user() called under set_fs(KERNEL_DS).
>
Although, it also looks like the problematic code can just be removed, since
always sizeof(compat_loff_t) == sizeof(loff_t). I'll send a patch to do that...
From: Eric Biggers <[email protected]>
Since the SNAPSHOT_GET_IMAGE_SIZE, SNAPSHOT_AVAIL_SWAP_SIZE, and
SNAPSHOT_ALLOC_SWAP_PAGE ioctls produce an loff_t result, and loff_t is
always 64-bit even in the compat case, there's no reason to have the
special compat handling for these ioctls. Just remove this unneeded
code so that these ioctls call into snapshot_ioctl() directly, doing
just the compat_ptr() conversion on the argument.
(This unnecessary code was also causing a KMSAN false positive.)
Signed-off-by: Eric Biggers <[email protected]>
---
kernel/power/user.c | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/kernel/power/user.c b/kernel/power/user.c
index 77438954cc2b..58ed9478787f 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -409,21 +409,7 @@ snapshot_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
switch (cmd) {
case SNAPSHOT_GET_IMAGE_SIZE:
case SNAPSHOT_AVAIL_SWAP_SIZE:
- case SNAPSHOT_ALLOC_SWAP_PAGE: {
- compat_loff_t __user *uoffset = compat_ptr(arg);
- loff_t offset;
- mm_segment_t old_fs;
- int err;
-
- old_fs = get_fs();
- set_fs(KERNEL_DS);
- err = snapshot_ioctl(file, cmd, (unsigned long) &offset);
- set_fs(old_fs);
- if (!err && put_user(offset, uoffset))
- err = -EFAULT;
- return err;
- }
-
+ case SNAPSHOT_ALLOC_SWAP_PAGE:
case SNAPSHOT_CREATE_IMAGE:
return snapshot_ioctl(file, cmd,
(unsigned long) compat_ptr(arg));
--
2.25.1
> > Looks like a KMSAN false positive? As far as I can tell, the memory is being
> > initialized by put_user() called under set_fs(KERNEL_DS).
Why? put_user() doesn't write to kernel memory, instead it copies a
value to the userspace.
That's why KMSAN performs kmsan_check_memory() on it.
It would actually be better if KMSAN printed an kernel-infoleak warning instead.
> Although, it also looks like the problematic code can just be removed, since
> always sizeof(compat_loff_t) == sizeof(loff_t). I'll send a patch to do that...
Thanks!
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
On Mon, Mar 09, 2020 at 12:53:28PM +0100, 'Alexander Potapenko' via syzkaller-bugs wrote:
> > > Looks like a KMSAN false positive? As far as I can tell, the memory is being
> > > initialized by put_user() called under set_fs(KERNEL_DS).
>
> Why? put_user() doesn't write to kernel memory, instead it copies a
> value to the userspace.
> That's why KMSAN performs kmsan_check_memory() on it.
> It would actually be better if KMSAN printed an kernel-infoleak warning instead.
When under set_fs(KERNEL_DS), the userspace access functions like put_user() and
copy_to_user() can write to kernel memory. It's discouraged and people have
been trying to get rid of uses of set_fs(), but a lot still remain, since
sometimes it's useful to allow code to operate on both user and kernel memory.
A common example is kernel_read().
>
> > Although, it also looks like the problematic code can just be removed, since
> > always sizeof(compat_loff_t) == sizeof(loff_t). I'll send a patch to do that...
>
> Thanks!
>
- Eric
On Mon, Mar 9, 2020 at 7:11 PM Eric Biggers <[email protected]> wrote:
>
> On Mon, Mar 09, 2020 at 12:53:28PM +0100, 'Alexander Potapenko' via syzkaller-bugs wrote:
> > > > Looks like a KMSAN false positive? As far as I can tell, the memory is being
> > > > initialized by put_user() called under set_fs(KERNEL_DS).
> >
> > Why? put_user() doesn't write to kernel memory, instead it copies a
> > value to the userspace.
> > That's why KMSAN performs kmsan_check_memory() on it.
> > It would actually be better if KMSAN printed an kernel-infoleak warning instead.
>
> When under set_fs(KERNEL_DS), the userspace access functions like put_user() and
> copy_to_user() can write to kernel memory. It's discouraged and people have
> been trying to get rid of uses of set_fs(), but a lot still remain, since
> sometimes it's useful to allow code to operate on both user and kernel memory.
> A common example is kernel_read().
Ah, you're right. We can simply check that the target address is in
the userspace before actually reporting the error.
On Sunday, March 8, 2020 4:27:01 AM CET Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> Since the SNAPSHOT_GET_IMAGE_SIZE, SNAPSHOT_AVAIL_SWAP_SIZE, and
> SNAPSHOT_ALLOC_SWAP_PAGE ioctls produce an loff_t result, and loff_t is
> always 64-bit even in the compat case, there's no reason to have the
> special compat handling for these ioctls. Just remove this unneeded
> code so that these ioctls call into snapshot_ioctl() directly, doing
> just the compat_ptr() conversion on the argument.
>
> (This unnecessary code was also causing a KMSAN false positive.)
>
> Signed-off-by: Eric Biggers <[email protected]>
> ---
> kernel/power/user.c | 16 +---------------
> 1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/kernel/power/user.c b/kernel/power/user.c
> index 77438954cc2b..58ed9478787f 100644
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -409,21 +409,7 @@ snapshot_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> switch (cmd) {
> case SNAPSHOT_GET_IMAGE_SIZE:
> case SNAPSHOT_AVAIL_SWAP_SIZE:
> - case SNAPSHOT_ALLOC_SWAP_PAGE: {
> - compat_loff_t __user *uoffset = compat_ptr(arg);
> - loff_t offset;
> - mm_segment_t old_fs;
> - int err;
> -
> - old_fs = get_fs();
> - set_fs(KERNEL_DS);
> - err = snapshot_ioctl(file, cmd, (unsigned long) &offset);
> - set_fs(old_fs);
> - if (!err && put_user(offset, uoffset))
> - err = -EFAULT;
> - return err;
> - }
> -
> + case SNAPSHOT_ALLOC_SWAP_PAGE:
> case SNAPSHOT_CREATE_IMAGE:
> return snapshot_ioctl(file, cmd,
> (unsigned long) compat_ptr(arg));
>
Applied as 5.7 material, thanks!