* [syzbot] KMSAN: uninit-value in pagecache_write @ 2022-11-07 9:10 syzbot 2022-11-07 9:46 ` Alexander Potapenko 0 siblings, 1 reply; 7+ messages in thread From: syzbot @ 2022-11-07 9:10 UTC (permalink / raw) To: adilger.kernel, glider, linux-ext4, linux-kernel, syzkaller-bugs, tytso Hello, syzbot found the following issue on: HEAD commit: 968c2729e576 x86: kmsan: fix comment in kmsan_shadow.c git tree: https://github.com/google/kmsan.git master console output: https://syzkaller.appspot.com/x/log.txt?x=11d01ad6880000 kernel config: https://syzkaller.appspot.com/x/.config?x=131312b26465c190 dashboard link: https://syzkaller.appspot.com/bug?extid=9767be679ef5016b6082 compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project.git 610139d2d9ce6746b3c617fb3e2f7886272d26ff), GNU ld (GNU Binutils for Debian) 2.35.2 userspace arch: i386 Unfortunately, I don't have any reproducer for this issue yet. Downloadable assets: disk image: https://storage.googleapis.com/syzbot-assets/c78ce21b953f/disk-968c2729.raw.xz vmlinux: https://storage.googleapis.com/syzbot-assets/22868d826804/vmlinux-968c2729.xz IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+9767be679ef5016b6082@syzkaller.appspotmail.com ===================================================== BUG: KMSAN: uninit-value in pagecache_write+0x655/0x720 fs/ext4/verity.c:91 pagecache_write+0x655/0x720 fs/ext4/verity.c:91 ext4_write_merkle_tree_block+0x84/0xa0 fs/ext4/verity.c:389 build_merkle_tree_level+0x972/0x1250 fs/verity/enable.c:121 build_merkle_tree fs/verity/enable.c:182 [inline] enable_verity+0xede/0x1920 fs/verity/enable.c:268 fsverity_ioctl_enable+0x895/0xab0 fs/verity/enable.c:392 __ext4_ioctl fs/ext4/ioctl.c:1572 [inline] ext4_ioctl+0x26dd/0x8c50 fs/ext4/ioctl.c:1606 ext4_compat_ioctl+0x702/0x800 fs/ext4/ioctl.c:1682 __do_compat_sys_ioctl fs/ioctl.c:968 [inline] __se_compat_sys_ioctl+0x781/0xfa0 fs/ioctl.c:910 __ia32_compat_sys_ioctl+0x8f/0xd0 fs/ioctl.c:910 do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline] __do_fast_syscall_32+0xa2/0x100 arch/x86/entry/common.c:178 do_fast_syscall_32+0x33/0x70 arch/x86/entry/common.c:203 do_SYSENTER_32+0x1b/0x20 arch/x86/entry/common.c:246 entry_SYSENTER_compat_after_hwframe+0x70/0x82 Local variable fsdata created at: pagecache_write+0x21c/0x720 fs/ext4/verity.c:85 ext4_write_merkle_tree_block+0x84/0xa0 fs/ext4/verity.c:389 CPU: 1 PID: 15121 Comm: syz-executor.3 Not tainted 6.0.0-rc5-syzkaller-48543-g968c2729e576 #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/11/2022 ===================================================== --- 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] 7+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in pagecache_write 2022-11-07 9:10 [syzbot] KMSAN: uninit-value in pagecache_write syzbot @ 2022-11-07 9:46 ` Alexander Potapenko 2022-11-07 17:56 ` Eric Biggers 0 siblings, 1 reply; 7+ messages in thread From: Alexander Potapenko @ 2022-11-07 9:46 UTC (permalink / raw) To: syzbot; +Cc: adilger.kernel, linux-ext4, linux-kernel, syzkaller-bugs, tytso On Mon, Nov 7, 2022 at 10:10 AM syzbot <syzbot+9767be679ef5016b6082@syzkaller.appspotmail.com> wrote: > > Hello, > > syzbot found the following issue on: > > HEAD commit: 968c2729e576 x86: kmsan: fix comment in kmsan_shadow.c > git tree: https://github.com/google/kmsan.git master > console output: https://syzkaller.appspot.com/x/log.txt?x=11d01ad6880000 > kernel config: https://syzkaller.appspot.com/x/.config?x=131312b26465c190 > dashboard link: https://syzkaller.appspot.com/bug?extid=9767be679ef5016b6082 > compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project.git 610139d2d9ce6746b3c617fb3e2f7886272d26ff), GNU ld (GNU Binutils for Debian) 2.35.2 > userspace arch: i386 > > Unfortunately, I don't have any reproducer for this issue yet. > > Downloadable assets: > disk image: https://storage.googleapis.com/syzbot-assets/c78ce21b953f/disk-968c2729.raw.xz > vmlinux: https://storage.googleapis.com/syzbot-assets/22868d826804/vmlinux-968c2729.xz > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+9767be679ef5016b6082@syzkaller.appspotmail.com > > ===================================================== > BUG: KMSAN: uninit-value in pagecache_write+0x655/0x720 fs/ext4/verity.c:91 > pagecache_write+0x655/0x720 fs/ext4/verity.c:91 > ext4_write_merkle_tree_block+0x84/0xa0 fs/ext4/verity.c:389 > build_merkle_tree_level+0x972/0x1250 fs/verity/enable.c:121 > build_merkle_tree fs/verity/enable.c:182 [inline] > enable_verity+0xede/0x1920 fs/verity/enable.c:268 > fsverity_ioctl_enable+0x895/0xab0 fs/verity/enable.c:392 > __ext4_ioctl fs/ext4/ioctl.c:1572 [inline] > ext4_ioctl+0x26dd/0x8c50 fs/ext4/ioctl.c:1606 > ext4_compat_ioctl+0x702/0x800 fs/ext4/ioctl.c:1682 > __do_compat_sys_ioctl fs/ioctl.c:968 [inline] > __se_compat_sys_ioctl+0x781/0xfa0 fs/ioctl.c:910 > __ia32_compat_sys_ioctl+0x8f/0xd0 fs/ioctl.c:910 > do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline] > __do_fast_syscall_32+0xa2/0x100 arch/x86/entry/common.c:178 > do_fast_syscall_32+0x33/0x70 arch/x86/entry/common.c:203 > do_SYSENTER_32+0x1b/0x20 arch/x86/entry/common.c:246 > entry_SYSENTER_compat_after_hwframe+0x70/0x82 > > Local variable fsdata created at: > pagecache_write+0x21c/0x720 fs/ext4/verity.c:85 > ext4_write_merkle_tree_block+0x84/0xa0 fs/ext4/verity.c:389 > > CPU: 1 PID: 15121 Comm: syz-executor.3 Not tainted 6.0.0-rc5-syzkaller-48543-g968c2729e576 #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/11/2022 > ===================================================== This is identical to other reports fixed in https://lore.kernel.org/lkml/20220915150417.722975-43-glider@google.com/ To fix the error, we need to initialize fsdata explicitly, because aops->write_begin is not guaranteed to do so: ============================================================================= ext4: initialize fsdata in pagecache_write() When aops->write_begin() does not initialize fsdata, KMSAN reports an error passing the latter to aops->write_end(). Fix this by unconditionally initializing fsdata. Fixes: c93d8f885809 ("ext4: add basic fs-verity support") Reported-by: syzbot+9767be679ef5016b6082@syzkaller.appspotmail.com Signed-off-by: Alexander Potapenko <glider@google.com> diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c index 3c640bd7ecaeb..30e3b65798b50 100644 --- a/fs/ext4/verity.c +++ b/fs/ext4/verity.c @@ -79,7 +79,7 @@ static int pagecache_write(struct inode *inode, const void *buf, size_t count, size_t n = min_t(size_t, count, PAGE_SIZE - offset_in_page(pos)); struct page *page; - void *fsdata; + void *fsdata = NULL; int res; res = aops->write_begin(NULL, mapping, pos, n, &page, &fsdata); ============================================================================= ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in pagecache_write 2022-11-07 9:46 ` Alexander Potapenko @ 2022-11-07 17:56 ` Eric Biggers 2022-11-07 18:14 ` Alexander Potapenko 0 siblings, 1 reply; 7+ messages in thread From: Eric Biggers @ 2022-11-07 17:56 UTC (permalink / raw) To: Alexander Potapenko Cc: syzbot, adilger.kernel, linux-ext4, linux-kernel, syzkaller-bugs, tytso On Mon, Nov 07, 2022 at 10:46:13AM +0100, 'Alexander Potapenko' via syzkaller-bugs wrote: > ext4: initialize fsdata in pagecache_write() > > When aops->write_begin() does not initialize fsdata, KMSAN reports > an error passing the latter to aops->write_end(). > > Fix this by unconditionally initializing fsdata. > > Fixes: c93d8f885809 ("ext4: add basic fs-verity support") > Reported-by: syzbot+9767be679ef5016b6082@syzkaller.appspotmail.com > Signed-off-by: Alexander Potapenko <glider@google.com> > > diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c > index 3c640bd7ecaeb..30e3b65798b50 100644 > --- a/fs/ext4/verity.c > +++ b/fs/ext4/verity.c > @@ -79,7 +79,7 @@ static int pagecache_write(struct inode *inode, > const void *buf, size_t count, > size_t n = min_t(size_t, count, > PAGE_SIZE - offset_in_page(pos)); > struct page *page; > - void *fsdata; > + void *fsdata = NULL; > int res; > > res = aops->write_begin(NULL, mapping, pos, n, &page, &fsdata); Are you sure that KMSAN should be reporting this? The uninitialized value is passed as a function parameter, but it's never actually used. Anyway, this patch doesn't hurt, I suppose. Can please you send it out as a formal patch to linux-ext4? It would be easy for people to miss this patch buried in this thread. Also, can you please send a patch to linux-f2fs-devel for the same code in fs/f2fs/verity.c? Thanks! - Eric ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in pagecache_write 2022-11-07 17:56 ` Eric Biggers @ 2022-11-07 18:14 ` Alexander Potapenko 2022-11-08 9:08 ` Alexander Potapenko 0 siblings, 1 reply; 7+ messages in thread From: Alexander Potapenko @ 2022-11-07 18:14 UTC (permalink / raw) To: Eric Biggers Cc: syzbot, adilger.kernel, linux-ext4, linux-kernel, syzkaller-bugs, tytso On Mon, Nov 7, 2022 at 6:56 PM Eric Biggers <ebiggers@kernel.org> wrote: > > On Mon, Nov 07, 2022 at 10:46:13AM +0100, 'Alexander Potapenko' via syzkaller-bugs wrote: > > ext4: initialize fsdata in pagecache_write() > > > > When aops->write_begin() does not initialize fsdata, KMSAN reports > > an error passing the latter to aops->write_end(). > > > > Fix this by unconditionally initializing fsdata. > > > > Fixes: c93d8f885809 ("ext4: add basic fs-verity support") > > Reported-by: syzbot+9767be679ef5016b6082@syzkaller.appspotmail.com > > Signed-off-by: Alexander Potapenko <glider@google.com> > > > > diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c > > index 3c640bd7ecaeb..30e3b65798b50 100644 > > --- a/fs/ext4/verity.c > > +++ b/fs/ext4/verity.c > > @@ -79,7 +79,7 @@ static int pagecache_write(struct inode *inode, > > const void *buf, size_t count, > > size_t n = min_t(size_t, count, > > PAGE_SIZE - offset_in_page(pos)); > > struct page *page; > > - void *fsdata; > > + void *fsdata = NULL; > > int res; > > > > res = aops->write_begin(NULL, mapping, pos, n, &page, &fsdata); > > Are you sure that KMSAN should be reporting this? The uninitialized value is > passed as a function parameter, but it's never actually used. To summarize what's written here: https://lore.kernel.org/lkml/20220701142310.2188015-44-glider@google.com/ : - this is UB from the C11 standpoint; - Linus despises UB in general, but expecting function arguments to be initialized is reasonable; - if a function ends up being a no-op, tools should not warn about uninits passed to it; - KMSAN only warns about functions that survive inlining. In addition, one cannot really rely on write_end() being consistent with write_begin(), in particular if fsdata was ignored initially and only appeared at a later point. > > Anyway, this patch doesn't hurt, I suppose. Can please you send it out as a > formal patch to linux-ext4? It would be easy for people to miss this patch > buried in this thread. Also, can you please send a patch to linux-f2fs-devel > for the same code in fs/f2fs/verity.c? Will do! > Thanks! > > - Eric -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Liana Sebastian Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in pagecache_write 2022-11-07 18:14 ` Alexander Potapenko @ 2022-11-08 9:08 ` Alexander Potapenko 2022-11-08 17:41 ` Eric Biggers 0 siblings, 1 reply; 7+ messages in thread From: Alexander Potapenko @ 2022-11-08 9:08 UTC (permalink / raw) To: Eric Biggers Cc: syzbot, adilger.kernel, linux-ext4, linux-kernel, syzkaller-bugs, tytso > > > > Anyway, this patch doesn't hurt, I suppose. Can please you send it out as a > > formal patch to linux-ext4? It would be easy for people to miss this patch > > buried in this thread. Also, can you please send a patch to linux-f2fs-devel > > for the same code in fs/f2fs/verity.c? > > Will do! Shall I also initialize fsdata here: $ git grep 'void \*fsdata;' fs/affs/file.c: void *fsdata; fs/ext4/verity.c: void *fsdata; fs/f2fs/verity.c: void *fsdata; fs/hfs/extent.c: void *fsdata; fs/hfsplus/extents.c: void *fsdata; fs/ocfs2/mmap.c: void *fsdata; ? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in pagecache_write 2022-11-08 9:08 ` Alexander Potapenko @ 2022-11-08 17:41 ` Eric Biggers 2022-11-10 11:01 ` Alexander Potapenko 0 siblings, 1 reply; 7+ messages in thread From: Eric Biggers @ 2022-11-08 17:41 UTC (permalink / raw) To: Alexander Potapenko Cc: syzbot, adilger.kernel, linux-ext4, linux-kernel, syzkaller-bugs, tytso On Tue, Nov 08, 2022 at 10:08:36AM +0100, 'Alexander Potapenko' via syzkaller-bugs wrote: > > > > > > Anyway, this patch doesn't hurt, I suppose. Can please you send it out as a > > > formal patch to linux-ext4? It would be easy for people to miss this patch > > > buried in this thread. Also, can you please send a patch to linux-f2fs-devel > > > for the same code in fs/f2fs/verity.c? > > > > Will do! > > Shall I also initialize fsdata here: > > $ git grep 'void \*fsdata;' > fs/affs/file.c: void *fsdata; > fs/ext4/verity.c: void *fsdata; > fs/f2fs/verity.c: void *fsdata; > fs/hfs/extent.c: void *fsdata; > fs/hfsplus/extents.c: void *fsdata; > fs/ocfs2/mmap.c: void *fsdata; Yes, it looks like they all need this. Except maybe ocfs2? It's hard to tell. - Eric ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [syzbot] KMSAN: uninit-value in pagecache_write 2022-11-08 17:41 ` Eric Biggers @ 2022-11-10 11:01 ` Alexander Potapenko 0 siblings, 0 replies; 7+ messages in thread From: Alexander Potapenko @ 2022-11-10 11:01 UTC (permalink / raw) To: Eric Biggers Cc: syzbot, adilger.kernel, linux-ext4, linux-kernel, syzkaller-bugs, tytso On Tue, Nov 8, 2022 at 6:41 PM Eric Biggers <ebiggers@kernel.org> wrote: > > On Tue, Nov 08, 2022 at 10:08:36AM +0100, 'Alexander Potapenko' via syzkaller-bugs wrote: > > > > > > > > Anyway, this patch doesn't hurt, I suppose. Can please you send it out as a > > > > formal patch to linux-ext4? It would be easy for people to miss this patch > > > > buried in this thread. Also, can you please send a patch to linux-f2fs-devel > > > > for the same code in fs/f2fs/verity.c? > > > > > > Will do! > > > > Shall I also initialize fsdata here: > > > > $ git grep 'void \*fsdata;' > > fs/affs/file.c: void *fsdata; > > fs/ext4/verity.c: void *fsdata; > > fs/f2fs/verity.c: void *fsdata; > > fs/hfs/extent.c: void *fsdata; > > fs/hfsplus/extents.c: void *fsdata; > > fs/ocfs2/mmap.c: void *fsdata; > > Yes, it looks like they all need this. Except maybe ocfs2? It's hard to tell. For ocfs2 the begin/end functions are always the same, so it's harder to mess fsdata up. Guess we can say for now that __ocfs2_page_mkwrite() never passes an uninitialized variable to another function. > - Eric -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Liana Sebastian Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-10 11:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-07 9:10 [syzbot] KMSAN: uninit-value in pagecache_write syzbot 2022-11-07 9:46 ` Alexander Potapenko 2022-11-07 17:56 ` Eric Biggers 2022-11-07 18:14 ` Alexander Potapenko 2022-11-08 9:08 ` Alexander Potapenko 2022-11-08 17:41 ` Eric Biggers 2022-11-10 11:01 ` Alexander Potapenko
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.