linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: torvalds@linux-foundation.org, ebiggers@kernel.org
Cc: dhowells@redhat.com, viro@zeniv.linux.org.uk,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] pipe: Fix missing mask update after pipe_wait() [ver #2]
Date: Thu, 05 Dec 2019 22:30:37 +0000	[thread overview]
Message-ID: <157558503716.10278.17734879104574600890.stgit@warthog.procyon.org.uk> (raw)
In-Reply-To: <157558502272.10278.8718685637610645781.stgit@warthog.procyon.org.uk>

Fix pipe_write() to not cache the ring index mask and max_usage as their
values are invalidated by calling pipe_wait() because the latter function
drops the pipe lock, thereby allowing F_SETPIPE_SZ change them.  Without
this, pipe_write() may subsequently miscalculate the array indices and pipe
fullness, leading to an oops like the following:

 BUG: KASAN: slab-out-of-bounds in pipe_write+0xc25/0xe10 fs/pipe.c:481
 Write of size 8 at addr ffff8880771167a8 by task syz-executor.3/7987
 ...
 CPU: 1 PID: 7987 Comm: syz-executor.3 Not tainted 5.4.0-rc2-syzkaller #0
 ...
 Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x113/0x167 lib/dump_stack.c:113
  print_address_description.constprop.8.cold.10+0x9/0x31d mm/kasan/report.c:374
  __kasan_report.cold.11+0x1b/0x3a mm/kasan/report.c:506
  kasan_report+0x12/0x20 mm/kasan/common.c:634
  __asan_report_store8_noabort+0x17/0x20 mm/kasan/generic_report.c:137
  pipe_write+0xc25/0xe10 fs/pipe.c:481
  call_write_iter include/linux/fs.h:1895 [inline]
  new_sync_write+0x3fd/0x7e0 fs/read_write.c:483
  __vfs_write+0x94/0x110 fs/read_write.c:496
  vfs_write+0x18a/0x520 fs/read_write.c:558
  ksys_write+0x105/0x220 fs/read_write.c:611
  __do_sys_write fs/read_write.c:623 [inline]
  __se_sys_write fs/read_write.c:620 [inline]
  __x64_sys_write+0x6e/0xb0 fs/read_write.c:620
  do_syscall_64+0xca/0x5d0 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

This is not a problem for pipe_read() as the mask is recalculated on each
pass of the loop, after pipe_wait() has been called.

Fixes: 8cefc107ca54 ("pipe: Use head and tail pointers for the ring, not cursor and length")
Reported-by: syzbot+838eb0878ffd51f27c41@syzkaller.appspotmail.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Eric Biggers <ebiggers@kernel.org>
---

 fs/pipe.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index da782ee251d2..8061b093140d 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -389,7 +389,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 {
 	struct file *filp = iocb->ki_filp;
 	struct pipe_inode_info *pipe = filp->private_data;
-	unsigned int head, max_usage, mask;
+	unsigned int head;
 	ssize_t ret = 0;
 	int do_wakeup = 0;
 	size_t total_len = iov_iter_count(from);
@@ -408,13 +408,12 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 	}
 
 	head = pipe->head;
-	max_usage = pipe->max_usage;
-	mask = pipe->ring_size - 1;
 
 	/* We try to merge small writes */
 	chars = total_len & (PAGE_SIZE-1); /* size of the last buffer */
 	if (!pipe_empty(head, pipe->tail) && chars != 0) {
-		struct pipe_buffer *buf = &pipe->bufs[(head - 1) & mask];
+		struct pipe_buffer *buf =
+			&pipe->bufs[(head - 1) & (pipe->ring_size - 1)];
 		int offset = buf->offset + buf->len;
 
 		if (pipe_buf_can_merge(buf) && offset + chars <= PAGE_SIZE) {
@@ -443,8 +442,9 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 		}
 
 		head = pipe->head;
-		if (!pipe_full(head, pipe->tail, max_usage)) {
-			struct pipe_buffer *buf = &pipe->bufs[head & mask];
+		if (!pipe_full(head, pipe->tail, pipe->max_usage)) {
+			struct pipe_buffer *buf =
+				&pipe->bufs[head & (pipe->ring_size - 1)];
 			struct page *page = pipe->tmp_page;
 			int copied;
 
@@ -465,7 +465,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 			spin_lock_irq(&pipe->wait.lock);
 
 			head = pipe->head;
-			if (pipe_full(head, pipe->tail, max_usage)) {
+			if (pipe_full(head, pipe->tail, pipe->max_usage)) {
 				spin_unlock_irq(&pipe->wait.lock);
 				continue;
 			}
@@ -484,7 +484,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 			kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
 
 			/* Insert it into the buffer array */
-			buf = &pipe->bufs[head & mask];
+			buf = &pipe->bufs[head & (pipe->ring_size - 1)];
 			buf->page = page;
 			buf->ops = &anon_pipe_buf_ops;
 			buf->offset = 0;
@@ -510,7 +510,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 				break;
 		}
 
-		if (!pipe_full(head, pipe->tail, max_usage))
+		if (!pipe_full(head, pipe->tail, pipe->max_usage))
 			continue;
 
 		/* Wait for buffer space to become available. */


  parent reply	other threads:[~2019-12-05 22:30 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-05 22:30 [PATCH 0/2] pipe: Fixes [ver #2] David Howells
2019-12-05 22:30 ` [PATCH 1/2] pipe: Remove assertion from pipe_poll() " David Howells
2019-12-05 22:30 ` David Howells [this message]
2019-12-05 23:58   ` [PATCH 2/2] pipe: Fix missing mask update after pipe_wait() " Linus Torvalds
2019-12-06 13:56 ` [PATCH 0/2] pipe: Fixes " David Sterba
2019-12-06 17:09   ` Linus Torvalds
2019-12-06 17:42     ` Linus Torvalds
2019-12-06 18:59       ` Linus Torvalds
2019-12-07 21:31         ` Akemi Yagi
2019-12-08 16:45           ` Akemi Yagi
2019-12-08 18:04             ` Linus Torvalds
2019-12-09  3:07               ` Linus Torvalds
2019-12-06 20:28   ` Linus Torvalds
2019-12-06 21:04     ` Linus Torvalds
2019-12-07  3:50       ` Linus Torvalds
2019-12-07  4:01         ` Linus Torvalds
2019-12-07 22:47         ` Linus Torvalds
2019-12-09  9:53           ` Vincent Guittot
2019-12-09 17:48             ` Linus Torvalds
2019-12-09 17:57               ` Akemi Yagi
2019-12-09 18:18                 ` Linus Torvalds
2019-12-09 18:24                   ` Linus Torvalds
2019-12-18 20:59                   ` Josh Triplett
2019-12-10  2:58               ` DJ Delorie
2019-12-10 14:38               ` Vincent Guittot
2019-12-10 17:39                 ` Linus Torvalds
2019-12-11 18:09               ` DJ Delorie
2019-12-11 18:59                 ` Linus Torvalds
2019-12-12 10:18           ` Konstantin Khlebnikov
2019-12-18 22:51             ` Linus Torvalds
2019-12-19  0:03               ` Josh Triplett
2019-12-19  0:14                 ` Josh Triplett
2019-12-19  0:51                   ` Linus Torvalds
2019-12-19  0:54                     ` Linus Torvalds
2019-12-19  7:56                   ` David Howells
2019-12-19 16:35                     ` Linus Torvalds
2019-12-11 20:55         ` David Howells
2019-12-12  1:28           ` Linus Torvalds
2019-12-12  7:34           ` David Howells
2019-12-09 14:55       ` David Sterba
2019-12-06 21:26   ` David Howells

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=157558503716.10278.17734879104574600890.stgit@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=ebiggers@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).