Hello,
syzbot found the following issue on:
HEAD commit: 622322f53c6d Merge tag 'mips-fixes_6.3_2' of git://git.ker..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12342880280000
kernel config: https://syzkaller.appspot.com/x/.config?x=fa4baf7c6b35b5d5
dashboard link: https://syzkaller.appspot.com/bug?extid=606f94dfeaaa45124c90
compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/8b5f31d96315/disk-622322f5.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/adca7dc8daae/vmlinux-622322f5.xz
kernel image: https://storage.googleapis.com/syzbot-assets/ed78ddc31ccb/bzImage-622322f5.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]
==================================================================
BUG: KCSAN: data-race in __filemap_remove_folio / folio_mapping
write to 0xffffea0004958618 of 8 bytes by task 17586 on cpu 1:
page_cache_delete mm/filemap.c:145 [inline]
__filemap_remove_folio+0x210/0x330 mm/filemap.c:225
invalidate_complete_folio2 mm/truncate.c:586 [inline]
invalidate_inode_pages2_range+0x506/0x790 mm/truncate.c:673
iomap_dio_complete+0x383/0x470 fs/iomap/direct-io.c:115
iomap_dio_rw+0x62/0x90 fs/iomap/direct-io.c:687
ext4_dio_write_iter fs/ext4/file.c:597 [inline]
ext4_file_write_iter+0x9e6/0x10e0 fs/ext4/file.c:708
do_iter_write+0x418/0x700 fs/read_write.c:861
vfs_iter_write+0x50/0x70 fs/read_write.c:902
iter_file_splice_write+0x456/0x7d0 fs/splice.c:778
do_splice_from fs/splice.c:856 [inline]
direct_splice_actor+0x84/0xa0 fs/splice.c:1022
splice_direct_to_actor+0x2ee/0x5f0 fs/splice.c:977
do_splice_direct+0x104/0x180 fs/splice.c:1065
do_sendfile+0x3b8/0x950 fs/read_write.c:1255
__do_sys_sendfile64 fs/read_write.c:1323 [inline]
__se_sys_sendfile64 fs/read_write.c:1309 [inline]
__x64_sys_sendfile64+0x110/0x150 fs/read_write.c:1309
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
read to 0xffffea0004958618 of 8 bytes by task 17568 on cpu 0:
folio_mapping+0x92/0x110 mm/util.c:774
folio_evictable mm/internal.h:156 [inline]
lru_add_fn+0x92/0x450 mm/swap.c:181
folio_batch_move_lru+0x21e/0x2f0 mm/swap.c:217
folio_batch_add_and_move mm/swap.c:234 [inline]
folio_add_lru+0xc9/0x130 mm/swap.c:517
filemap_add_folio+0xfc/0x150 mm/filemap.c:954
page_cache_ra_unbounded+0x15e/0x2e0 mm/readahead.c:251
do_page_cache_ra mm/readahead.c:300 [inline]
page_cache_ra_order mm/readahead.c:560 [inline]
ondemand_readahead+0x550/0x6c0 mm/readahead.c:682
page_cache_sync_ra+0x284/0x2a0 mm/readahead.c:709
page_cache_sync_readahead include/linux/pagemap.h:1214 [inline]
filemap_get_pages+0x257/0xea0 mm/filemap.c:2598
filemap_read+0x223/0x680 mm/filemap.c:2693
generic_file_read_iter+0x76/0x320 mm/filemap.c:2840
ext4_file_read_iter+0x1cc/0x290
call_read_iter include/linux/fs.h:1845 [inline]
generic_file_splice_read+0xe3/0x290 fs/splice.c:402
do_splice_to fs/splice.c:885 [inline]
splice_direct_to_actor+0x25a/0x5f0 fs/splice.c:956
do_splice_direct+0x104/0x180 fs/splice.c:1065
do_sendfile+0x3b8/0x950 fs/read_write.c:1255
__do_sys_sendfile64 fs/read_write.c:1323 [inline]
__se_sys_sendfile64 fs/read_write.c:1309 [inline]
__x64_sys_sendfile64+0x110/0x150 fs/read_write.c:1309
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
value changed: 0xffff88810a98f7b0 -> 0x0000000000000000
Reported by Kernel Concurrency Sanitizer on:
CPU: 0 PID: 17568 Comm: syz-executor.2 Not tainted 6.3.0-rc7-syzkaller-00191-g622322f53c6d #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/14/2023
==================================================================
---
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 [email protected].
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
On Mon, 24 Apr 2023 at 09:19, syzbot
<[email protected]> wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 622322f53c6d Merge tag 'mips-fixes_6.3_2' of git://git.ker..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=12342880280000
> kernel config: https://syzkaller.appspot.com/x/.config?x=fa4baf7c6b35b5d5
> dashboard link: https://syzkaller.appspot.com/bug?extid=606f94dfeaaa45124c90
> compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
>
> Unfortunately, I don't have any reproducer for this issue yet.
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/8b5f31d96315/disk-622322f5.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/adca7dc8daae/vmlinux-622322f5.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/ed78ddc31ccb/bzImage-622322f5.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: [email protected]
If I am reading this correctly, it can lead to NULL derefs in
folio_mapping() if folio->mapping is read twice. I think
folio->mapping reads/writes need to use READ/WRITE_ONCE if racy.
> ==================================================================
> BUG: KCSAN: data-race in __filemap_remove_folio / folio_mapping
>
> write to 0xffffea0004958618 of 8 bytes by task 17586 on cpu 1:
> page_cache_delete mm/filemap.c:145 [inline]
> __filemap_remove_folio+0x210/0x330 mm/filemap.c:225
> invalidate_complete_folio2 mm/truncate.c:586 [inline]
> invalidate_inode_pages2_range+0x506/0x790 mm/truncate.c:673
> iomap_dio_complete+0x383/0x470 fs/iomap/direct-io.c:115
> iomap_dio_rw+0x62/0x90 fs/iomap/direct-io.c:687
> ext4_dio_write_iter fs/ext4/file.c:597 [inline]
> ext4_file_write_iter+0x9e6/0x10e0 fs/ext4/file.c:708
> do_iter_write+0x418/0x700 fs/read_write.c:861
> vfs_iter_write+0x50/0x70 fs/read_write.c:902
> iter_file_splice_write+0x456/0x7d0 fs/splice.c:778
> do_splice_from fs/splice.c:856 [inline]
> direct_splice_actor+0x84/0xa0 fs/splice.c:1022
> splice_direct_to_actor+0x2ee/0x5f0 fs/splice.c:977
> do_splice_direct+0x104/0x180 fs/splice.c:1065
> do_sendfile+0x3b8/0x950 fs/read_write.c:1255
> __do_sys_sendfile64 fs/read_write.c:1323 [inline]
> __se_sys_sendfile64 fs/read_write.c:1309 [inline]
> __x64_sys_sendfile64+0x110/0x150 fs/read_write.c:1309
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> read to 0xffffea0004958618 of 8 bytes by task 17568 on cpu 0:
> folio_mapping+0x92/0x110 mm/util.c:774
> folio_evictable mm/internal.h:156 [inline]
> lru_add_fn+0x92/0x450 mm/swap.c:181
> folio_batch_move_lru+0x21e/0x2f0 mm/swap.c:217
> folio_batch_add_and_move mm/swap.c:234 [inline]
> folio_add_lru+0xc9/0x130 mm/swap.c:517
> filemap_add_folio+0xfc/0x150 mm/filemap.c:954
> page_cache_ra_unbounded+0x15e/0x2e0 mm/readahead.c:251
> do_page_cache_ra mm/readahead.c:300 [inline]
> page_cache_ra_order mm/readahead.c:560 [inline]
> ondemand_readahead+0x550/0x6c0 mm/readahead.c:682
> page_cache_sync_ra+0x284/0x2a0 mm/readahead.c:709
> page_cache_sync_readahead include/linux/pagemap.h:1214 [inline]
> filemap_get_pages+0x257/0xea0 mm/filemap.c:2598
> filemap_read+0x223/0x680 mm/filemap.c:2693
> generic_file_read_iter+0x76/0x320 mm/filemap.c:2840
> ext4_file_read_iter+0x1cc/0x290
> call_read_iter include/linux/fs.h:1845 [inline]
> generic_file_splice_read+0xe3/0x290 fs/splice.c:402
> do_splice_to fs/splice.c:885 [inline]
> splice_direct_to_actor+0x25a/0x5f0 fs/splice.c:956
> do_splice_direct+0x104/0x180 fs/splice.c:1065
> do_sendfile+0x3b8/0x950 fs/read_write.c:1255
> __do_sys_sendfile64 fs/read_write.c:1323 [inline]
> __se_sys_sendfile64 fs/read_write.c:1309 [inline]
> __x64_sys_sendfile64+0x110/0x150 fs/read_write.c:1309
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> value changed: 0xffff88810a98f7b0 -> 0x0000000000000000
>
> Reported by Kernel Concurrency Sanitizer on:
> CPU: 0 PID: 17568 Comm: syz-executor.2 Not tainted 6.3.0-rc7-syzkaller-00191-g622322f53c6d #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/14/2023
> ==================================================================
>
>
> ---
> 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 [email protected].
>
> 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 [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/000000000000d0737c05fa0fd499%40google.com.
On Mon, Apr 24, 2023 at 09:38:43AM +0200, Dmitry Vyukov wrote:
> On Mon, 24 Apr 2023 at 09:19, syzbot
> <[email protected]> wrote:
> If I am reading this correctly, it can lead to NULL derefs in
> folio_mapping() if folio->mapping is read twice. I think
> folio->mapping reads/writes need to use READ/WRITE_ONCE if racy.
You aren't reading it correctly.
mapping = folio->mapping;
if ((unsigned long)mapping & PAGE_MAPPING_FLAGS)
return NULL;
return mapping;
The racing write is storing NULL. So it might return NULL or it might
return the old mapping, or it might return NULL. Either way, the caller
has to be prepared for NULL to be returned.
It's a false posiive, but probably worth silencing with a READ_ONCE().
On Mon, 24 Apr 2023 at 15:21, Matthew Wilcox <[email protected]> wrote:
>
> On Mon, Apr 24, 2023 at 09:38:43AM +0200, Dmitry Vyukov wrote:
> > On Mon, 24 Apr 2023 at 09:19, syzbot
> > <[email protected]> wrote:
> > If I am reading this correctly, it can lead to NULL derefs in
> > folio_mapping() if folio->mapping is read twice. I think
> > folio->mapping reads/writes need to use READ/WRITE_ONCE if racy.
>
> You aren't reading it correctly.
>
> mapping = folio->mapping;
> if ((unsigned long)mapping & PAGE_MAPPING_FLAGS)
> return NULL;
>
> return mapping;
>
> The racing write is storing NULL. So it might return NULL or it might
> return the old mapping, or it might return NULL. Either way, the caller
> has to be prepared for NULL to be returned.
>
> It's a false posiive, but probably worth silencing with a READ_ONCE().
Yes, but the end of the function does not limit effects of races. I
think this can still crash on NULL deref.
The simplest example would be to compile this:
struct address_space *folio_mapping(struct folio *folio)
{
...
mapping = folio->mapping;
if ((unsigned long)mapping & PAGE_MAPPING_FLAGS)
return NULL;
return mapping;
}
ret = !mapping_unevictable(folio_mapping(folio)) &&
!folio_test_mlocked(folio);
static inline bool mapping_unevictable(struct address_space *mapping)
{
return mapping && test_bit(AS_UNEVICTABLE, &mapping->flags);
}
to this:
if (!((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) && folio->mapping)
if (test_bit(AS_UNEVICTABLE, &folio->mapping->flags))
which does crash.
On Mon, Apr 24, 2023 at 03:49:04PM +0200, Dmitry Vyukov wrote:
> On Mon, 24 Apr 2023 at 15:21, Matthew Wilcox <[email protected]> wrote:
> >
> > On Mon, Apr 24, 2023 at 09:38:43AM +0200, Dmitry Vyukov wrote:
> > > On Mon, 24 Apr 2023 at 09:19, syzbot
> > > <[email protected]> wrote:
> > > If I am reading this correctly, it can lead to NULL derefs in
> > > folio_mapping() if folio->mapping is read twice. I think
> > > folio->mapping reads/writes need to use READ/WRITE_ONCE if racy.
> >
> > You aren't reading it correctly.
> >
> > mapping = folio->mapping;
> > if ((unsigned long)mapping & PAGE_MAPPING_FLAGS)
> > return NULL;
> >
> > return mapping;
> >
> > The racing write is storing NULL. So it might return NULL or it might
> > return the old mapping, or it might return NULL. Either way, the caller
> > has to be prepared for NULL to be returned.
> >
> > It's a false posiive, but probably worth silencing with a READ_ONCE().
>
> Yes, but the end of the function does not limit effects of races. I
I thought it did. I was under the impression that the compiler was not
allowed to extract loads from within the function and move them outside.
Maybe that changed since C99.
> to this:
>
> if (!((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) && folio->mapping)
> if (test_bit(AS_UNEVICTABLE, &folio->mapping->flags))
>
> which does crash.
Yes, if the compiler is allowed to do that, then that's a possibility.
On Mon, 24 Apr 2023 at 16:10, Matthew Wilcox <[email protected]> wrote:
>
> On Mon, Apr 24, 2023 at 03:49:04PM +0200, Dmitry Vyukov wrote:
> > On Mon, 24 Apr 2023 at 15:21, Matthew Wilcox <[email protected]> wrote:
> > >
> > > On Mon, Apr 24, 2023 at 09:38:43AM +0200, Dmitry Vyukov wrote:
> > > > On Mon, 24 Apr 2023 at 09:19, syzbot
> > > > <[email protected]> wrote:
> > > > If I am reading this correctly, it can lead to NULL derefs in
> > > > folio_mapping() if folio->mapping is read twice. I think
> > > > folio->mapping reads/writes need to use READ/WRITE_ONCE if racy.
> > >
> > > You aren't reading it correctly.
> > >
> > > mapping = folio->mapping;
> > > if ((unsigned long)mapping & PAGE_MAPPING_FLAGS)
> > > return NULL;
> > >
> > > return mapping;
> > >
> > > The racing write is storing NULL. So it might return NULL or it might
> > > return the old mapping, or it might return NULL. Either way, the caller
> > > has to be prepared for NULL to be returned.
> > >
> > > It's a false posiive, but probably worth silencing with a READ_ONCE().
> >
> > Yes, but the end of the function does not limit effects of races. I
>
> I thought it did. I was under the impression that the compiler was not
> allowed to extract loads from within the function and move them outside.
> Maybe that changed since C99.
>
> > to this:
> >
> > if (!((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) && folio->mapping)
> > if (test_bit(AS_UNEVICTABLE, &folio->mapping->flags))
> >
> > which does crash.
>
> Yes, if the compiler is allowed to do that, then that's a possibility.
C11/C++11 simply say any data race renders behavior of the whole
program undefined. There is no discussion about values, functions,
anything else.
Before that there was no notion of data races, so it wasn't possible
to talk about possible effects and restrict them. But I don't think
there ever was an intention to do any practical restrictions around
function boundaries. That would mean that inlining can only run as the
latest optimization pass, which would inhibit tons of optimizations.
Users would throw such a compiler away.
please test data-race in __filemap_remove_folio / folio_mapping
#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> please test data-race in __filemap_remove_folio / folio_mapping
>
> #syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
This crash does not have a reproducer. I cannot test it.