lkml.org 
[lkml]   [2019]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Subject[PATCH 2/2] pipe: Fix missing mask update after pipe_wait() [ver #2]
From
Date
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. */
\
 
 \ /
  Last update: 2019-12-05 23:31    [W:0.092 / U:28.892 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site