All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jann Horn <jannh@google.com>
To: Al Viro <viro@zeniv.linux.org.uk>,
	Miklos Szeredi <miklos@szeredi.hu>, Jens Axboe <axboe@kernel.dk>,
	Jens Axboe <axboe@fb.com>,
	jannh@google.com
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Kees Cook <keescook@chromium.org>,
	Eric Biggers <ebiggers@google.com>
Subject: [PATCH 1/2] splice: don't merge into linked buffers
Date: Mon, 15 Oct 2018 17:04:18 +0200	[thread overview]
Message-ID: <20181015150420.2096-1-jannh@google.com> (raw)

Before this patch, it was possible for two pipes to affect each other after
data had been transferred between them with tee():

============
$ cat tee_test.c

int main(void) {
  int pipe_a[2];
  if (pipe(pipe_a)) err(1, "pipe");
  int pipe_b[2];
  if (pipe(pipe_b)) err(1, "pipe");
  if (write(pipe_a[1], "abcd", 4) != 4) err(1, "write");
  if (tee(pipe_a[0], pipe_b[1], 2, 0) != 2) err(1, "tee");
  if (write(pipe_b[1], "xx", 2) != 2) err(1, "write");

  char buf[5];
  if (read(pipe_a[0], buf, 4) != 4) err(1, "read");
  buf[4] = 0;
  printf("got back: '%s'\n", buf);
}
$ gcc -o tee_test tee_test.c
$ ./tee_test
got back: 'abxx'
$
============

Fix it by explicitly marking buffers as mergeable and clearing that flag in
splice_pipe_to_pipe() and link_pipe().

Cc: <stable@vger.kernel.org>
Fixes: 7c77f0b3f920 ("splice: implement pipe to pipe splicing")
Fixes: 70524490ee2e ("[PATCH] splice: add support for sys_tee()")
Signed-off-by: Jann Horn <jannh@google.com>
---
Cleanup in the next patch, to simplify backporting.

 fs/pipe.c                 | 5 +++--
 fs/splice.c               | 6 ++++++
 include/linux/pipe_fs_i.h | 8 ++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index bdc5d3c0977d..4e2eee77f855 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -379,7 +379,8 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 		struct pipe_buffer *buf = pipe->bufs + lastbuf;
 		int offset = buf->offset + buf->len;
 
-		if (buf->ops->can_merge && offset + chars <= PAGE_SIZE) {
+		if (buf->ops->can_merge && offset + chars <= PAGE_SIZE &&
+		    (buf->flags & PIPE_BUF_FLAG_MAYMERGE)) {
 			ret = pipe_buf_confirm(pipe, buf);
 			if (ret)
 				goto out;
@@ -439,7 +440,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 			buf->ops = &anon_pipe_buf_ops;
 			buf->offset = 0;
 			buf->len = copied;
-			buf->flags = 0;
+			buf->flags = PIPE_BUF_FLAG_MAYMERGE;
 			if (is_packetized(filp)) {
 				buf->ops = &packet_pipe_buf_ops;
 				buf->flags = PIPE_BUF_FLAG_PACKET;
diff --git a/fs/splice.c b/fs/splice.c
index b3daa971f597..111977c80dfd 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1593,6 +1593,9 @@ static int splice_pipe_to_pipe(struct pipe_inode_info *ipipe,
 			 */
 			obuf->flags &= ~PIPE_BUF_FLAG_GIFT;
 
+			/* We can't merge data into a buffer we don't own. */
+			obuf->flags &= ~PIPE_BUF_FLAG_MAYMERGE;
+
 			obuf->len = len;
 			opipe->nrbufs++;
 			ibuf->offset += obuf->len;
@@ -1667,6 +1670,9 @@ static int link_pipe(struct pipe_inode_info *ipipe,
 		 */
 		obuf->flags &= ~PIPE_BUF_FLAG_GIFT;
 
+		/* We can't merge data into a buffer we don't own. */
+		obuf->flags &= ~PIPE_BUF_FLAG_MAYMERGE;
+
 		if (obuf->len > len)
 			obuf->len = len;
 
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index 5a3bb3b7c9ad..8893711f9171 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -8,6 +8,14 @@
 #define PIPE_BUF_FLAG_ATOMIC	0x02	/* was atomically mapped */
 #define PIPE_BUF_FLAG_GIFT	0x04	/* page is a gift */
 #define PIPE_BUF_FLAG_PACKET	0x08	/* read() as a packet */
+/*
+ * Set this flag if the generic pipe read/write may coalesce data into an
+ * existing buffer. If this is not set, a new pipe page segment is always used
+ * for new data.
+ * When pipe data is copied by reference (as in the tee() syscall), this flag
+ * must be cleared on the copy.
+ */
+#define PIPE_BUF_FLAG_MAYMERGE	0x10
 
 /**
  *	struct pipe_buffer - a linux kernel pipe buffer
-- 
2.19.0.605.g01d371f741-goog


             reply	other threads:[~2018-10-15 15:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-15 15:04 Jann Horn [this message]
2018-10-15 15:04 ` [PATCH 2/2] pipe: remove ->can_merge from pipe_buf_operations Jann Horn
2018-10-15 21:00 ` [PATCH 1/2] splice: don't merge into linked buffers Eric Biggers
2018-10-15 21:13   ` Jann Horn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181015150420.2096-1-jannh@google.com \
    --to=jannh@google.com \
    --cc=axboe@fb.com \
    --cc=axboe@kernel.dk \
    --cc=ebiggers@google.com \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.