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@xxxxxxxxxx> > > 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@xxxxxxxxxxxxxxxxxxxxxxxxx > > 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
