> Tested on: > > commit: aaec5a95 pipe_read: don't wake up the writer if the pi.. > git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > console output: https://syzkaller.appspot.com/x/log.txt?x=169ac43f980000 > kernel config: https://syzkaller.appspot.com/x/.config?x=8d5a2956e94d7972 > dashboard link: https://syzkaller.appspot.com/bug?extid=62262fdc0e01d99573fc > compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40 > patch: https://syzkaller.appspot.com/x/patch.diff?x=17803c4c580000 > Here is a "just in case" by me: the patch which made sure to only look at head + tail with the lock held. #syz test: upstream aaec5a95d59615523db03dd53c2052f0a87beea7 diff --git a/fs/pipe.c b/fs/pipe.c index 82fede0f2111..7eedcef2811e 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -210,11 +210,20 @@ static const struct pipe_buf_operations anon_pipe_buf_ops = { /* Done while waiting without holding the pipe lock - thus the READ_ONCE() */ static inline bool pipe_readable(const struct pipe_inode_info *pipe) { - unsigned int head = READ_ONCE(pipe->head); - unsigned int tail = READ_ONCE(pipe->tail); - unsigned int writers = READ_ONCE(pipe->writers); + return !READ_ONCE(pipe->isempty) || !READ_ONCE(pipe->writers); +} + +static inline void pipe_recalc_state(struct pipe_inode_info *pipe) +{ + pipe->isempty = pipe_empty(pipe->head, pipe->tail); + pipe->isfull = pipe_full(pipe->head, pipe->tail, pipe->max_usage); +} - return !pipe_empty(head, tail) || !writers; +static inline void pipe_update_head(struct pipe_inode_info *pipe, + unsigned int head) +{ + pipe->head = ++head; + pipe_recalc_state(pipe); } static inline unsigned int pipe_update_tail(struct pipe_inode_info *pipe, @@ -244,6 +253,7 @@ static inline unsigned int pipe_update_tail(struct pipe_inode_info *pipe, * without the spinlock - the mutex is enough. */ pipe->tail = ++tail; + pipe_recalc_state(pipe); return tail; } @@ -417,12 +427,7 @@ static inline int is_packetized(struct file *file) /* Done while waiting without holding the pipe lock - thus the READ_ONCE() */ static inline bool pipe_writable(const struct pipe_inode_info *pipe) { - unsigned int head = READ_ONCE(pipe->head); - unsigned int tail = READ_ONCE(pipe->tail); - unsigned int max_usage = READ_ONCE(pipe->max_usage); - - return !pipe_full(head, tail, max_usage) || - !READ_ONCE(pipe->readers); + return !READ_ONCE(pipe->isfull) || !READ_ONCE(pipe->readers); } static ssize_t @@ -524,7 +529,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) * it, either the reader will consume it or it'll still * be there for the next write. */ - pipe->head = head + 1; + pipe_update_head(pipe, head); /* Insert it into the buffer array */ buf = &pipe->bufs[head & mask]; @@ -549,10 +554,9 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) if (!iov_iter_count(from)) break; - } - if (!pipe_full(head, pipe->tail, pipe->max_usage)) continue; + } /* Wait for buffer space to become available. */ if ((filp->f_flags & O_NONBLOCK) || diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h index 8ff23bf5a819..d4b7539399b5 100644 --- a/include/linux/pipe_fs_i.h +++ b/include/linux/pipe_fs_i.h @@ -69,6 +69,8 @@ struct pipe_inode_info { unsigned int r_counter; unsigned int w_counter; bool poll_usage; + bool isempty; + bool isfull; #ifdef CONFIG_WATCH_QUEUE bool note_loss; #endif
