On Fri, 5 Feb 2021 at 18:00, syzbot <syzbot+2b6452167d85a022bc6f@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > Hello, > > syzbot found the following issue on: > > HEAD commit: 2ab38c17 mailmap: remove the "repo-abbrev" comment > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=130e19b4d00000 > kernel config: https://syzkaller.appspot.com/x/.config?x=38728258f37833e3 > dashboard link: https://syzkaller.appspot.com/bug?extid=2b6452167d85a022bc6f > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project.git 913f6005669cfb590c99865a90bc51ed0983d09d) > > 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+2b6452167d85a022bc6f@xxxxxxxxxxxxxxxxxxxxxxxxx > > ================================================================== > BUG: KCSAN: data-race in blk_stat_add / blk_stat_timer_fn > > write to 0xffffe8ffffd35c80 of 8 bytes by interrupt on cpu 0: > blk_rq_stat_init block/blk-stat.c:24 [inline] > blk_stat_timer_fn+0x349/0x410 block/blk-stat.c:95 > call_timer_fn+0x2e/0x240 kernel/time/timer.c:1417 > expire_timers+0x116/0x260 kernel/time/timer.c:1462 > __run_timers+0x338/0x3d0 kernel/time/timer.c:1731 > run_timer_softirq+0x19/0x30 kernel/time/timer.c:1744 > __do_softirq+0x13c/0x2c3 kernel/softirq.c:343 > asm_call_irq_on_stack+0xf/0x20 > __run_on_irqstack arch/x86/include/asm/irq_stack.h:26 [inline] > run_on_irqstack_cond arch/x86/include/asm/irq_stack.h:77 [inline] > do_softirq_own_stack+0x32/0x40 arch/x86/kernel/irq_64.c:77 > invoke_softirq kernel/softirq.c:226 [inline] > __irq_exit_rcu+0xb4/0xc0 kernel/softirq.c:420 > sysvec_apic_timer_interrupt+0x74/0x90 arch/x86/kernel/apic/apic.c:1096 > asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:628 > > read to 0xffffe8ffffd35c80 of 8 bytes by interrupt on cpu 1: > blk_rq_stat_add block/blk-stat.c:46 [inline] > blk_stat_add+0x13d/0x230 block/blk-stat.c:74 > __blk_mq_end_request+0x142/0x230 block/blk-mq.c:546 > scsi_end_request+0x2a6/0x470 drivers/scsi/scsi_lib.c:604 > scsi_io_completion+0x104/0xfb0 drivers/scsi/scsi_lib.c:969 > scsi_finish_command+0x263/0x2b0 drivers/scsi/scsi.c:214 > scsi_softirq_done+0xdf/0x440 drivers/scsi/scsi_lib.c:1449 > blk_done_softirq+0x145/0x190 block/blk-mq.c:588 > __do_softirq+0x13c/0x2c3 kernel/softirq.c:343 > asm_call_irq_on_stack+0xf/0x20 > __run_on_irqstack arch/x86/include/asm/irq_stack.h:26 [inline] > run_on_irqstack_cond arch/x86/include/asm/irq_stack.h:77 [inline] > do_softirq_own_stack+0x32/0x40 arch/x86/kernel/irq_64.c:77 > invoke_softirq kernel/softirq.c:226 [inline] > __irq_exit_rcu+0xb4/0xc0 kernel/softirq.c:420 > common_interrupt+0xb5/0x130 arch/x86/kernel/irq.c:239 > asm_common_interrupt+0x1e/0x40 arch/x86/include/asm/idtentry.h:619 > check_access kernel/kcsan/core.c:633 [inline] > __tsan_read1+0x156/0x180 kernel/kcsan/core.c:839 > tomoyo_get_mode security/tomoyo/util.c:1003 [inline] > tomoyo_init_request_info+0xfc/0x160 security/tomoyo/util.c:1031 > tomoyo_path_perm+0x8b/0x330 security/tomoyo/file.c:815 > tomoyo_inode_getattr+0x18/0x20 security/tomoyo/tomoyo.c:123 > security_inode_getattr+0x7f/0xd0 security/security.c:1280 > vfs_getattr fs/stat.c:121 [inline] > vfs_fstat+0x45/0x390 fs/stat.c:146 > __do_sys_newfstat fs/stat.c:386 [inline] > __se_sys_newfstat+0x35/0x240 fs/stat.c:383 > __x64_sys_newfstat+0x2d/0x40 fs/stat.c:383 > do_syscall_64+0x39/0x80 arch/x86/entry/common.c:46 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > Reported by Kernel Concurrency Sanitizer on: > CPU: 1 PID: 18199 Comm: modprobe Not tainted 5.11.0-rc5-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > ================================================================== I've been looking at some data races in block/. For this one I was wondering if there are any requirements for the stats counters? E.g. do they have to be somewhat consistent, or does it not matter at all? Because as-is, with concurrent update and aggregation (followed by reinit) of the per-CPU counters, the values in blk_rq_stat can become quite inconsistent. I wanted to throw together a fix for this, but wasn't sure what the level of tolerable errors for these counters is appropriate. I thought of 3 options: 1. Just add more data_race() around them and accept whatever inaccuracies we get due to the data races. 2. Add a per-CPU spinlock. This should be uncontended unless the timer fires too often. 3. Use per-CPU seqlock. Not sure this buys us much because the timer also resets the per-CPU counters and has to be serialized with other potential updaters. Thanks, -- Marco
