lkml.org 
[lkml]   [2019]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: kernel BUG at fs/pipe.c:LINE!
Date
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

\
 
 \ /
  Last update: 2019-12-05 14:07    [W:0.039 / U:2.580 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site