KMSAN: uninit-value in snapshot_compat_ioctl

KMSAN: uninit-value in snapshot_compat_ioctl

[syzbot]

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: syzbot+af962bf9e7e27bccd025@syzkaller.appspotmail.com

=====================================================
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 syzkaller@googlegroups.com.

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

Re: KMSAN: uninit-value in snapshot_compat_ioctl

[Eric Biggers]

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: syzbot+af962bf9e7e27bccd025@syzkaller.appspotmail.com
> 
> =====================================================
> 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).

Re: KMSAN: uninit-value in snapshot_compat_ioctl

[Eric Biggers]

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: syzbot+af962bf9e7e27bccd025@syzkaller.appspotmail.com
> > 
> > =====================================================
> > 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...

[PATCH] PM / hibernate: Remove unnecessary compat ioctl overrides

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 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

Re: KMSAN: uninit-value in snapshot_compat_ioctl

[Alexander Potapenko]

> > 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

Re: KMSAN: uninit-value in snapshot_compat_ioctl

[Eric Biggers]

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

Re: KMSAN: uninit-value in snapshot_compat_ioctl

[Alexander Potapenko]

On Mon, Mar 9, 2020 at 7:11 PM Eric Biggers <ebiggers@kernel.org> 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.

Re: [PATCH] PM / hibernate: Remove unnecessary compat ioctl overrides

[Rafael J. Wysocki]

On Sunday, March 8, 2020 4:27:01 AM CET Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> 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 <ebiggers@google.com>
> ---
>  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!