From 8924feff66f35fe22ce77aafe3f21eb8e5cff881 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 17 Sep 2016 20:44:45 -0400 Subject: splice: lift pipe_lock out of splice_to_pipe() * splice_to_pipe() stops at pipe overflow and does *not* take pipe_lock * ->splice_read() instances do the same * vmsplice_to_pipe() and do_splice() (ultimate callers of splice_to_pipe()) arrange for waiting, looping, etc. themselves. That should make pipe_lock the outermost one. Unfortunately, existing rules for the amount passed by vmsplice_to_pipe() and do_splice() are quite ugly _and_ userland code can be easily broken by changing those. It's not even "no more than the maximal capacity of this pipe" - it's "once we'd fed pipe->nr_buffers pages into the pipe, leave instead of waiting". Considering how poorly these rules are documented, let's try "wait for some space to appear, unless given SPLICE_F_NONBLOCK, then push into pipe and if we run into overflow, we are done". Signed-off-by: Al Viro --- fs/fuse/dev.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'fs/fuse/dev.c') diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index a94d2ed81ab4..eaf56c6e9123 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -1364,7 +1364,6 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos, goto out; ret = 0; - pipe_lock(pipe); if (!pipe->readers) { send_sig(SIGPIPE, current, 0); @@ -1400,7 +1399,6 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos, } out_unlock: - pipe_unlock(pipe); if (do_wakeup) { smp_mb(); -- cgit v1.2.3-55-g7522 From d82718e348fee15dbce8f578ff2588982b7cc7ca Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 17 Sep 2016 22:56:25 -0400 Subject: fuse_dev_splice_read(): switch to add_to_pipe() Signed-off-by: Al Viro --- fs/fuse/dev.c | 46 +++++++++------------------------------------- 1 file changed, 9 insertions(+), 37 deletions(-) (limited to 'fs/fuse/dev.c') diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index eaf56c6e9123..0a6a808d561c 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -1342,9 +1342,8 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos, struct pipe_inode_info *pipe, size_t len, unsigned int flags) { - int ret; + int total, ret; int page_nr = 0; - int do_wakeup = 0; struct pipe_buffer *bufs; struct fuse_copy_state cs; struct fuse_dev *fud = fuse_get_dev(in); @@ -1363,50 +1362,23 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos, if (ret < 0) goto out; - ret = 0; - - if (!pipe->readers) { - send_sig(SIGPIPE, current, 0); - if (!ret) - ret = -EPIPE; - goto out_unlock; - } - if (pipe->nrbufs + cs.nr_segs > pipe->buffers) { ret = -EIO; - goto out_unlock; + goto out; } - while (page_nr < cs.nr_segs) { - int newbuf = (pipe->curbuf + pipe->nrbufs) & (pipe->buffers - 1); - struct pipe_buffer *buf = pipe->bufs + newbuf; - - buf->page = bufs[page_nr].page; - buf->offset = bufs[page_nr].offset; - buf->len = bufs[page_nr].len; + for (ret = total = 0; page_nr < cs.nr_segs; total += ret) { /* * Need to be careful about this. Having buf->ops in module * code can Oops if the buffer persists after module unload. */ - buf->ops = &nosteal_pipe_buf_ops; - - pipe->nrbufs++; - page_nr++; - ret += buf->len; - - if (pipe->files) - do_wakeup = 1; - } - -out_unlock: - - if (do_wakeup) { - smp_mb(); - if (waitqueue_active(&pipe->wait)) - wake_up_interruptible(&pipe->wait); - kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN); + bufs[page_nr].ops = &nosteal_pipe_buf_ops; + ret = add_to_pipe(pipe, &bufs[page_nr++]); + if (unlikely(ret < 0)) + break; } - + if (total) + ret = total; out: for (; page_nr < cs.nr_segs; page_nr++) put_page(bufs[page_nr].page); -- cgit v1.2.3-55-g7522 From 7bf2d1df80822ec056363627e2014990f068f7aa Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Tue, 27 Sep 2016 10:45:12 +0200 Subject: pipe: add pipe_buf_get() helper Signed-off-by: Miklos Szeredi Signed-off-by: Al Viro --- fs/fuse/dev.c | 2 +- fs/splice.c | 4 ++-- include/linux/pipe_fs_i.h | 11 +++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) (limited to 'fs/fuse/dev.c') diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 0a6a808d561c..b50220c75132 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -1963,7 +1963,7 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, pipe->curbuf = (pipe->curbuf + 1) & (pipe->buffers - 1); pipe->nrbufs--; } else { - ibuf->ops->get(pipe, ibuf); + pipe_buf_get(pipe, ibuf); *obuf = *ibuf; obuf->flags &= ~PIPE_BUF_FLAG_GIFT; obuf->len = rem; diff --git a/fs/splice.c b/fs/splice.c index 0df907ba46ed..188a386bf379 100644 --- a/fs/splice.c +++ b/fs/splice.c @@ -1596,7 +1596,7 @@ retry: * Get a reference to this pipe buffer, * so we can copy the contents over. */ - ibuf->ops->get(ipipe, ibuf); + pipe_buf_get(ipipe, ibuf); *obuf = *ibuf; /* @@ -1668,7 +1668,7 @@ static int link_pipe(struct pipe_inode_info *ipipe, * Get a reference to this pipe buffer, * so we can copy the contents over. */ - ibuf->ops->get(ipipe, ibuf); + pipe_buf_get(ipipe, ibuf); obuf = opipe->bufs + nbuf; *obuf = *ibuf; diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h index 24f5470d3944..10876f3cb3da 100644 --- a/include/linux/pipe_fs_i.h +++ b/include/linux/pipe_fs_i.h @@ -115,6 +115,17 @@ struct pipe_buf_operations { void (*get)(struct pipe_inode_info *, struct pipe_buffer *); }; +/** + * pipe_buf_get - get a reference to a pipe_buffer + * @pipe: the pipe that the buffer belongs to + * @buf: the buffer to get a reference to + */ +static inline void pipe_buf_get(struct pipe_inode_info *pipe, + struct pipe_buffer *buf) +{ + buf->ops->get(pipe, buf); +} + /* Differs from PIPE_BUF in that PIPE_SIZE is the length of the actual memory allocation, whereas PIPE_BUF makes atomicity guarantees. */ #define PIPE_SIZE PAGE_SIZE -- cgit v1.2.3-55-g7522 From a779638cf622f069a484e8802134cca3c6c71415 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Tue, 27 Sep 2016 10:45:12 +0200 Subject: pipe: add pipe_buf_release() helper Signed-off-by: Miklos Szeredi Signed-off-by: Al Viro --- fs/fuse/dev.c | 7 +++---- fs/pipe.c | 5 ++--- fs/splice.c | 17 +++++------------ include/linux/pipe_fs_i.h | 14 ++++++++++++++ lib/iov_iter.c | 4 +--- 5 files changed, 25 insertions(+), 22 deletions(-) (limited to 'fs/fuse/dev.c') diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index b50220c75132..d82414a1f936 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -1985,10 +1985,9 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, ret = fuse_dev_do_write(fud, &cs, len); - for (idx = 0; idx < nbuf; idx++) { - struct pipe_buffer *buf = &bufs[idx]; - buf->ops->release(pipe, buf); - } + for (idx = 0; idx < nbuf; idx++) + pipe_buf_release(pipe, &bufs[idx]); + out: kfree(bufs); return ret; diff --git a/fs/pipe.c b/fs/pipe.c index 4ebe6b2e5217..67b5f1923835 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -299,8 +299,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) } if (!buf->len) { - buf->ops = NULL; - ops->release(pipe, buf); + pipe_buf_release(pipe, buf); curbuf = (curbuf + 1) & (pipe->buffers - 1); pipe->curbuf = curbuf; pipe->nrbufs = --bufs; @@ -664,7 +663,7 @@ void free_pipe_info(struct pipe_inode_info *pipe) for (i = 0; i < pipe->buffers; i++) { struct pipe_buffer *buf = pipe->bufs + i; if (buf->ops) - buf->ops->release(pipe, buf); + pipe_buf_release(pipe, buf); } if (pipe->tmp_page) __free_page(pipe->tmp_page); diff --git a/fs/splice.c b/fs/splice.c index 188a386bf379..ae90cd1d2999 100644 --- a/fs/splice.c +++ b/fs/splice.c @@ -238,8 +238,7 @@ ssize_t add_to_pipe(struct pipe_inode_info *pipe, struct pipe_buffer *buf) pipe->nrbufs++; return buf->len; } - buf->ops->release(pipe, buf); - buf->ops = NULL; + pipe_buf_release(pipe, buf); return ret; } EXPORT_SYMBOL(add_to_pipe); @@ -516,7 +515,6 @@ static int splice_from_pipe_feed(struct pipe_inode_info *pipe, struct splice_des while (pipe->nrbufs) { struct pipe_buffer *buf = pipe->bufs + pipe->curbuf; - const struct pipe_buf_operations *ops = buf->ops; sd->len = buf->len; if (sd->len > sd->total_len) @@ -542,8 +540,7 @@ static int splice_from_pipe_feed(struct pipe_inode_info *pipe, struct splice_des sd->total_len -= ret; if (!buf->len) { - buf->ops = NULL; - ops->release(pipe, buf); + pipe_buf_release(pipe, buf); pipe->curbuf = (pipe->curbuf + 1) & (pipe->buffers - 1); pipe->nrbufs--; if (pipe->files) @@ -789,11 +786,9 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out, while (ret) { struct pipe_buffer *buf = pipe->bufs + pipe->curbuf; if (ret >= buf->len) { - const struct pipe_buf_operations *ops = buf->ops; ret -= buf->len; buf->len = 0; - buf->ops = NULL; - ops->release(pipe, buf); + pipe_buf_release(pipe, buf); pipe->curbuf = (pipe->curbuf + 1) & (pipe->buffers - 1); pipe->nrbufs--; if (pipe->files) @@ -1032,10 +1027,8 @@ out_release: for (i = 0; i < pipe->buffers; i++) { struct pipe_buffer *buf = pipe->bufs + i; - if (buf->ops) { - buf->ops->release(pipe, buf); - buf->ops = NULL; - } + if (buf->ops) + pipe_buf_release(pipe, buf); } if (!bytes) diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h index 10876f3cb3da..d24fa6da6ae3 100644 --- a/include/linux/pipe_fs_i.h +++ b/include/linux/pipe_fs_i.h @@ -126,6 +126,20 @@ static inline void pipe_buf_get(struct pipe_inode_info *pipe, buf->ops->get(pipe, buf); } +/** + * pipe_buf_release - put a reference to a pipe_buffer + * @pipe: the pipe that the buffer belongs to + * @buf: the buffer to put a reference to + */ +static inline void pipe_buf_release(struct pipe_inode_info *pipe, + struct pipe_buffer *buf) +{ + const struct pipe_buf_operations *ops = buf->ops; + + buf->ops = NULL; + ops->release(pipe, buf); +} + /* Differs from PIPE_BUF in that PIPE_SIZE is the length of the actual memory allocation, whereas PIPE_BUF makes atomicity guarantees. */ #define PIPE_SIZE PAGE_SIZE diff --git a/lib/iov_iter.c b/lib/iov_iter.c index 659eaafcde65..48b8c27acabb 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -709,9 +709,7 @@ static void pipe_advance(struct iov_iter *i, size_t size) int unused = (pipe->curbuf + pipe->nrbufs) & (pipe->buffers - 1); /* [curbuf,unused) is in use. Free [idx,unused) */ while (idx != unused) { - buf = &pipe->bufs[idx]; - buf->ops->release(pipe, buf); - buf->ops = NULL; + pipe_buf_release(pipe, &pipe->bufs[idx]); idx = next_idx(idx, pipe); pipe->nrbufs--; } -- cgit v1.2.3-55-g7522 From fba597db4218ac324eee34b64736ea94829c95bf Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Tue, 27 Sep 2016 10:45:12 +0200 Subject: pipe: add pipe_buf_confirm() helper Signed-off-by: Miklos Szeredi Signed-off-by: Al Viro --- fs/fuse/dev.c | 4 ++-- fs/pipe.c | 8 +++----- fs/splice.c | 4 ++-- include/linux/pipe_fs_i.h | 12 +++++++++++- 4 files changed, 18 insertions(+), 10 deletions(-) (limited to 'fs/fuse/dev.c') diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index d82414a1f936..e5d5cc922c70 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -728,7 +728,7 @@ static int fuse_copy_fill(struct fuse_copy_state *cs) struct pipe_buffer *buf = cs->pipebufs; if (!cs->write) { - err = buf->ops->confirm(cs->pipe, buf); + err = pipe_buf_confirm(cs->pipe, buf); if (err) return err; @@ -828,7 +828,7 @@ static int fuse_try_move_page(struct fuse_copy_state *cs, struct page **pagep) fuse_copy_finish(cs); - err = buf->ops->confirm(cs->pipe, buf); + err = pipe_buf_confirm(cs->pipe, buf); if (err) return err; diff --git a/fs/pipe.c b/fs/pipe.c index 67b5f1923835..4fc422f0dea8 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -267,7 +267,6 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) if (bufs) { int curbuf = pipe->curbuf; struct pipe_buffer *buf = pipe->bufs + curbuf; - const struct pipe_buf_operations *ops = buf->ops; size_t chars = buf->len; size_t written; int error; @@ -275,7 +274,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) if (chars > total_len) chars = total_len; - error = ops->confirm(pipe, buf); + error = pipe_buf_confirm(pipe, buf); if (error) { if (!ret) ret = error; @@ -382,11 +381,10 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) int lastbuf = (pipe->curbuf + pipe->nrbufs - 1) & (pipe->buffers - 1); struct pipe_buffer *buf = pipe->bufs + lastbuf; - const struct pipe_buf_operations *ops = buf->ops; int offset = buf->offset + buf->len; - if (ops->can_merge && offset + chars <= PAGE_SIZE) { - ret = ops->confirm(pipe, buf); + if (buf->ops->can_merge && offset + chars <= PAGE_SIZE) { + ret = pipe_buf_confirm(pipe, buf); if (ret) goto out; diff --git a/fs/splice.c b/fs/splice.c index ae90cd1d2999..aa38901a4f10 100644 --- a/fs/splice.c +++ b/fs/splice.c @@ -520,7 +520,7 @@ static int splice_from_pipe_feed(struct pipe_inode_info *pipe, struct splice_des if (sd->len > sd->total_len) sd->len = sd->total_len; - ret = buf->ops->confirm(pipe, buf); + ret = pipe_buf_confirm(pipe, buf); if (unlikely(ret)) { if (ret == -ENODATA) ret = 0; @@ -759,7 +759,7 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out, if (idx == pipe->buffers - 1) idx = -1; - ret = buf->ops->confirm(pipe, buf); + ret = pipe_buf_confirm(pipe, buf); if (unlikely(ret)) { if (ret == -ENODATA) ret = 0; diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h index d24fa6da6ae3..654413334537 100644 --- a/include/linux/pipe_fs_i.h +++ b/include/linux/pipe_fs_i.h @@ -140,6 +140,17 @@ static inline void pipe_buf_release(struct pipe_inode_info *pipe, ops->release(pipe, buf); } +/** + * pipe_buf_confirm - verify contents of the pipe buffer + * @pipe: the pipe that the buffer belongs to + * @buf: the buffer to confirm + */ +static inline int pipe_buf_confirm(struct pipe_inode_info *pipe, + struct pipe_buffer *buf) +{ + return buf->ops->confirm(pipe, buf); +} + /* Differs from PIPE_BUF in that PIPE_SIZE is the length of the actual memory allocation, whereas PIPE_BUF makes atomicity guarantees. */ #define PIPE_SIZE PAGE_SIZE @@ -154,7 +165,6 @@ extern unsigned long pipe_user_pages_hard; extern unsigned long pipe_user_pages_soft; int pipe_proc_fn(struct ctl_table *, int, void __user *, size_t *, loff_t *); - /* Drop the inode semaphore and wait for a pipe event, atomically */ void pipe_wait(struct pipe_inode_info *pipe); -- cgit v1.2.3-55-g7522 From ca76f5b6bdbdc50af0d7b98cfcf7a2be7e95eb3d Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Tue, 27 Sep 2016 10:45:12 +0200 Subject: pipe: add pipe_buf_steal() helper Signed-off-by: Miklos Szeredi Signed-off-by: Al Viro --- drivers/char/virtio_console.c | 2 +- fs/fuse/dev.c | 2 +- include/linux/pipe_fs_i.h | 11 +++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) (limited to 'fs/fuse/dev.c') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 5da47e26a012..8114744bf30c 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -889,7 +889,7 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf, return 0; /* Try lock this page */ - if (buf->ops->steal(pipe, buf) == 0) { + if (pipe_buf_steal(pipe, buf) == 0) { /* Get reference and unlock page for moving */ get_page(buf->page); unlock_page(buf->page); diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index e5d5cc922c70..17a706da8931 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -841,7 +841,7 @@ static int fuse_try_move_page(struct fuse_copy_state *cs, struct page **pagep) if (cs->len != PAGE_SIZE) goto out_fallback; - if (buf->ops->steal(cs->pipe, buf) != 0) + if (pipe_buf_steal(cs->pipe, buf) != 0) goto out_fallback; newpage = buf->page; diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h index 654413334537..bddccf0159bb 100644 --- a/include/linux/pipe_fs_i.h +++ b/include/linux/pipe_fs_i.h @@ -151,6 +151,17 @@ static inline int pipe_buf_confirm(struct pipe_inode_info *pipe, return buf->ops->confirm(pipe, buf); } +/** + * pipe_buf_steal - attempt to take ownership of a pipe_buffer + * @pipe: the pipe that the buffer belongs to + * @buf: the buffer to attempt to steal + */ +static inline int pipe_buf_steal(struct pipe_inode_info *pipe, + struct pipe_buffer *buf) +{ + return buf->ops->steal(pipe, buf); +} + /* Differs from PIPE_BUF in that PIPE_SIZE is the length of the actual memory allocation, whereas PIPE_BUF makes atomicity guarantees. */ #define PIPE_SIZE PAGE_SIZE -- cgit v1.2.3-55-g7522