* kernel BUG at fs/pipe.c:LINE! @ 2019-12-02 6:45 syzbot 2019-12-05 5:40 ` Eric Biggers 0 siblings, 1 reply; 5+ messages in thread From: syzbot @ 2019-12-02 6:45 UTC (permalink / raw) To: amit, arnd, dhowells, gregkh, jannh, linux-fsdevel, linux-kernel, miklos, rostedt, syzkaller-bugs, viro, virtualization, willy Hello, syzbot found the following crash on: HEAD commit: b94ae8ad Merge tag 'seccomp-v5.5-rc1' of git://git.kernel... git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=1387ab12e00000 kernel config: https://syzkaller.appspot.com/x/.config?x=ff560c3de405258c dashboard link: https://syzkaller.appspot.com/bug?extid=d37abaade33a934f16f2 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12945c41e00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=161e202ee00000 The bug was bisected to: commit 8cefc107ca54c8b06438b7dc9cc08bc0a11d5b98 Author: David Howells <dhowells@redhat.com> Date: Fri Nov 15 13:30:32 2019 +0000 pipe: Use head and tail pointers for the ring, not cursor and length bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=118cce96e00000 final crash: https://syzkaller.appspot.com/x/report.txt?x=138cce96e00000 console output: https://syzkaller.appspot.com/x/log.txt?x=158cce96e00000 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+d37abaade33a934f16f2@syzkaller.appspotmail.com Fixes: 8cefc107ca54 ("pipe: Use head and tail pointers for the ring, not cursor and length") ------------[ cut here ]------------ kernel BUG at fs/pipe.c:582! invalid opcode: 0000 [#1] PREEMPT SMP KASAN CPU: 0 PID: 9433 Comm: syz-executor802 Not tainted 5.4.0-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:pipe_poll+0x37f/0x400 fs/pipe.c:582 Code: ff 85 db 75 09 e8 b1 ee b5 ff 41 83 ce 08 e8 a8 ee b5 ff 44 89 f0 48 83 c4 10 5b 41 5c 41 5d 41 5e 41 5f 5d c3 e8 91 ee b5 ff <0f> 0b e8 ca 40 f3 ff e9 ed fc ff ff e8 c0 40 f3 ff e9 b3 fd ff ff RSP: 0018:ffff888093e3f698 EFLAGS: 00010293 RAX: ffff8880a96083c0 RBX: ffff88809af6e800 RCX: ffffffff81beed8a RDX: 0000000000000000 RSI: ffffffff81beefaf RDI: 0000000000000004 RBP: ffff888093e3f6d0 R08: ffff8880a96083c0 R09: ffff8880a9608c50 R10: fffffbfff146e220 R11: ffffffff8a371107 R12: ffff88809c770d40 R13: 00000000fffffffa R14: 0000000000000010 R15: 00000000000001f6 FS: 00007fc5aa3cb700(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000020200000 CR3: 000000009b5a1000 CR4: 00000000001406f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: vfs_poll include/linux/poll.h:90 [inline] do_select+0x922/0x16f0 fs/select.c:534 core_sys_select+0x53c/0x8c0 fs/select.c:677 do_pselect.constprop.0+0x199/0x1e0 fs/select.c:759 __do_sys_pselect6 fs/select.c:784 [inline] __se_sys_pselect6 fs/select.c:769 [inline] __x64_sys_pselect6+0x1fc/0x2e0 fs/select.c:769 do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x44a629 Code: e8 5c b3 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 2b cc fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007fc5aa3cada8 EFLAGS: 00000246 ORIG_RAX: 000000000000010e RAX: ffffffffffffffda RBX: 00000000006dbc38 RCX: 000000000044a629 RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000040 RBP: 00000000006dbc30 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000020000140 R11: 0000000000000246 R12: 00000000006dbc3c R13: 00007fffcaf6d1bf R14: 00007fc5aa3cb9c0 R15: 20c49ba5e353f7cf Modules linked in: ---[ end trace 1c441dd64ff48137 ]--- RIP: 0010:pipe_poll+0x37f/0x400 fs/pipe.c:582 Code: ff 85 db 75 09 e8 b1 ee b5 ff 41 83 ce 08 e8 a8 ee b5 ff 44 89 f0 48 83 c4 10 5b 41 5c 41 5d 41 5e 41 5f 5d c3 e8 91 ee b5 ff <0f> 0b e8 ca 40 f3 ff e9 ed fc ff ff e8 c0 40 f3 ff e9 b3 fd ff ff RSP: 0018:ffff888093e3f698 EFLAGS: 00010293 RAX: ffff8880a96083c0 RBX: ffff88809af6e800 RCX: ffffffff81beed8a RDX: 0000000000000000 RSI: ffffffff81beefaf RDI: 0000000000000004 RBP: ffff888093e3f6d0 R08: ffff8880a96083c0 R09: ffff8880a9608c50 R10: fffffbfff146e220 R11: ffffffff8a371107 R12: ffff88809c770d40 R13: 00000000fffffffa R14: 0000000000000010 R15: 00000000000001f6 FS: 00007fc5aa3cb700(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000020200000 CR3: 000000009b5a1000 CR4: 00000000001406f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 --- 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. For information about bisection process see: https://goo.gl/tpsmEJ#bisection syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: kernel BUG at fs/pipe.c:LINE! 2019-12-02 6:45 kernel BUG at fs/pipe.c:LINE! syzbot @ 2019-12-05 5:40 ` Eric Biggers 2019-12-05 7:28 ` Eric Biggers ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Eric Biggers @ 2019-12-05 5:40 UTC (permalink / raw) To: dhowells Cc: amit, arnd, syzbot, gregkh, jannh, linux-fsdevel, linux-kernel, miklos, rostedt, syzkaller-bugs, viro, virtualization, willy David, On Sun, Dec 01, 2019 at 10:45:08PM -0800, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit: b94ae8ad Merge tag 'seccomp-v5.5-rc1' of git://git.kernel... > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=1387ab12e00000 > kernel config: https://syzkaller.appspot.com/x/.config?x=ff560c3de405258c > dashboard link: https://syzkaller.appspot.com/bug?extid=d37abaade33a934f16f2 > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12945c41e00000 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=161e202ee00000 > > The bug was bisected to: > > commit 8cefc107ca54c8b06438b7dc9cc08bc0a11d5b98 > Author: David Howells <dhowells@redhat.com> > Date: Fri Nov 15 13:30:32 2019 +0000 > > pipe: Use head and tail pointers for the ring, not cursor and length > > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=118cce96e00000 > final crash: https://syzkaller.appspot.com/x/report.txt?x=138cce96e00000 > console output: https://syzkaller.appspot.com/x/log.txt?x=158cce96e00000 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+d37abaade33a934f16f2@syzkaller.appspotmail.com > Fixes: 8cefc107ca54 ("pipe: Use head and tail pointers for the ring, not > cursor and length") > > ------------[ cut here ]------------ > kernel BUG at fs/pipe.c:582! This same BUG_ON() crashed my system during normal use, no syzkaller involved at all, on mainline 937d6eefc7. Can you please take a look? This syzbot report has a reproducer so that might be the easiest place to start. - Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: kernel BUG at fs/pipe.c:LINE! 2019-12-05 5:40 ` Eric Biggers @ 2019-12-05 7:28 ` Eric Biggers 2019-12-05 13:06 ` David Howells 2019-12-05 13:06 ` David Howells 2 siblings, 0 replies; 5+ messages in thread From: Eric Biggers @ 2019-12-05 7:28 UTC (permalink / raw) To: dhowells Cc: amit, arnd, syzbot, gregkh, jannh, linux-fsdevel, linux-kernel, miklos, rostedt, syzkaller-bugs, viro, virtualization, willy On Wed, Dec 04, 2019 at 09:40:23PM -0800, Eric Biggers wrote: > David, > > On Sun, Dec 01, 2019 at 10:45:08PM -0800, syzbot wrote: > > Hello, > > > > syzbot found the following crash on: > > > > HEAD commit: b94ae8ad Merge tag 'seccomp-v5.5-rc1' of git://git.kernel... > > git tree: upstream > > console output: https://syzkaller.appspot.com/x/log.txt?x=1387ab12e00000 > > kernel config: https://syzkaller.appspot.com/x/.config?x=ff560c3de405258c > > dashboard link: https://syzkaller.appspot.com/bug?extid=d37abaade33a934f16f2 > > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12945c41e00000 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=161e202ee00000 > > > > The bug was bisected to: > > > > commit 8cefc107ca54c8b06438b7dc9cc08bc0a11d5b98 > > Author: David Howells <dhowells@redhat.com> > > Date: Fri Nov 15 13:30:32 2019 +0000 > > > > pipe: Use head and tail pointers for the ring, not cursor and length > > > > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=118cce96e00000 > > final crash: https://syzkaller.appspot.com/x/report.txt?x=138cce96e00000 > > console output: https://syzkaller.appspot.com/x/log.txt?x=158cce96e00000 > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+d37abaade33a934f16f2@syzkaller.appspotmail.com > > Fixes: 8cefc107ca54 ("pipe: Use head and tail pointers for the ring, not > > cursor and length") > > > > ------------[ cut here ]------------ > > kernel BUG at fs/pipe.c:582! > > This same BUG_ON() crashed my system during normal use, no syzkaller involved at > all, on mainline 937d6eefc7. Can you please take a look? This syzbot report > has a reproducer so that might be the easiest place to start. > > - Eric Code is: static __poll_t pipe_poll(struct file *filp, poll_table *wait) { __poll_t mask; struct pipe_inode_info *pipe = filp->private_data; unsigned int head = READ_ONCE(pipe->head); unsigned int tail = READ_ONCE(pipe->tail); poll_wait(filp, &pipe->wait, wait); BUG_ON(pipe_occupancy(head, tail) > pipe->ring_size); It's not holding the pipe mutex, right? So 'head', 'tail' and 'ring_size' can all be changed concurrently, and they aren't read atomically with respect to each other. How do you propose to implement poll() correctly with the new head + tail approach? Just take the mutex? - Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: kernel BUG at fs/pipe.c:LINE! 2019-12-05 5:40 ` Eric Biggers 2019-12-05 7:28 ` Eric Biggers @ 2019-12-05 13:06 ` David Howells 2019-12-05 13:06 ` David Howells 2 siblings, 0 replies; 5+ messages in thread From: David Howells @ 2019-12-05 13:06 UTC (permalink / raw) To: Eric Biggers Cc: willy, arnd, jannh, syzbot, amit, syzkaller-bugs, linux-kernel, rostedt, virtualization, dhowells, miklos, viro, gregkh, linux-fsdevel Eric Biggers <ebiggers@kernel.org> wrote: > static __poll_t > pipe_poll(struct file *filp, poll_table *wait) > { > __poll_t mask; > struct pipe_inode_info *pipe = filp->private_data; > unsigned int head = READ_ONCE(pipe->head); > unsigned int tail = READ_ONCE(pipe->tail); > > poll_wait(filp, &pipe->wait, wait); > > BUG_ON(pipe_occupancy(head, tail) > pipe->ring_size); > > It's not holding the pipe mutex, right? So 'head', 'tail' and 'ring_size' can > all be changed concurrently, and they aren't read atomically with respect to > each other. > > How do you propose to implement poll() correctly with the new head + tail > approach? Just take the mutex? Firstly, the BUG_ON() check probably isn't necessary here - the same issue with occupancy being seen to be greater than the queue depth existed previously (there was no locking around the read of pipe->nrbufs and pipe->buffers). I added a sanity check. Secondly, it should be possible to make it such that just the spinlock suffices. The following few patches make the main pipe read/write routines use the spinlock so as not to be interfered with by notification insertion. I didn't roll the spinlock out to splice and suchlike since I prohibit splicing to a notifications pipe because of the iov_iter_revert() fun. David ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: kernel BUG at fs/pipe.c:LINE! 2019-12-05 5:40 ` Eric Biggers 2019-12-05 7:28 ` Eric Biggers 2019-12-05 13:06 ` David Howells @ 2019-12-05 13:06 ` David Howells 2 siblings, 0 replies; 5+ messages in thread From: David Howells @ 2019-12-05 13:06 UTC (permalink / raw) To: Eric Biggers Cc: dhowells, amit, arnd, syzbot, gregkh, jannh, linux-fsdevel, linux-kernel, miklos, rostedt, syzkaller-bugs, viro, virtualization, willy Eric Biggers <ebiggers@kernel.org> wrote: > static __poll_t > pipe_poll(struct file *filp, poll_table *wait) > { > __poll_t mask; > struct pipe_inode_info *pipe = filp->private_data; > unsigned int head = READ_ONCE(pipe->head); > unsigned int tail = READ_ONCE(pipe->tail); > > poll_wait(filp, &pipe->wait, wait); > > BUG_ON(pipe_occupancy(head, tail) > pipe->ring_size); > > It's not holding the pipe mutex, right? So 'head', 'tail' and 'ring_size' can > all be changed concurrently, and they aren't read atomically with respect to > each other. > > How do you propose to implement poll() correctly with the new head + tail > approach? Just take the mutex? Firstly, the BUG_ON() check probably isn't necessary here - the same issue with occupancy being seen to be greater than the queue depth existed previously (there was no locking around the read of pipe->nrbufs and pipe->buffers). I added a sanity check. Secondly, it should be possible to make it such that just the spinlock suffices. The following few patches make the main pipe read/write routines use the spinlock so as not to be interfered with by notification insertion. I didn't roll the spinlock out to splice and suchlike since I prohibit splicing to a notifications pipe because of the iov_iter_revert() fun. David ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-12-05 13:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-02 6:45 kernel BUG at fs/pipe.c:LINE! syzbot 2019-12-05 5:40 ` Eric Biggers 2019-12-05 7:28 ` Eric Biggers 2019-12-05 13:06 ` David Howells 2019-12-05 13:06 ` David Howells
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.