Skip to content
Snippets Groups Projects
Commit abb87f77 authored by Jeff Vander Stoep's avatar Jeff Vander Stoep Committed by Pat Tjin
Browse files

pipe: iovec: Fix OOB read in pipe_read()


Previous upstream *stable* fix 14f81062 was incomplete.

A local process can trigger a system crash with an OOB read on buf.
This occurs when the state of buf gets out of sync. After an error in
pipe_iov_copy_to_user() read_pipe may exit having updated buf->offset
but not buf->len. Upon retrying pipe_read() while in
pipe_iov_copy_to_user() *remaining will be larger than the space left
after buf->offset e.g. *remaing = PAGE_SIZE, buf->len = PAGE_SIZE,
buf->offset = 0x300.

This is fixed by not updating the state of buf->offset until after the
full copy is completed, similar to how pipe_write() is implemented.

For stable kernels < 3.16.

Bug: 27721803
Change-Id: Iefffbcc6cfd159dba69c31bcd98c6d5c1f21ff2e
Signed-off-by: default avatarJeff Vander Stoep <jeffv@google.com>
parent 62576fad
No related branches found
No related tags found
No related merge requests found
...@@ -400,7 +400,7 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov, ...@@ -400,7 +400,7 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov,
const struct pipe_buf_operations *ops = buf->ops; const struct pipe_buf_operations *ops = buf->ops;
void *addr; void *addr;
size_t chars = buf->len, remaining; size_t chars = buf->len, remaining;
int error, atomic; int error, atomic, offset;
if (chars > total_len) if (chars > total_len)
chars = total_len; chars = total_len;
...@@ -414,9 +414,10 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov, ...@@ -414,9 +414,10 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov,
atomic = !iov_fault_in_pages_write(iov, chars); atomic = !iov_fault_in_pages_write(iov, chars);
remaining = chars; remaining = chars;
offset = buf->offset;
redo: redo:
addr = ops->map(pipe, buf, atomic); addr = ops->map(pipe, buf, atomic);
error = pipe_iov_copy_to_user(iov, addr, &buf->offset, error = pipe_iov_copy_to_user(iov, addr, &offset,
&remaining, atomic); &remaining, atomic);
ops->unmap(pipe, buf, addr); ops->unmap(pipe, buf, addr);
if (unlikely(error)) { if (unlikely(error)) {
...@@ -432,6 +433,7 @@ redo: ...@@ -432,6 +433,7 @@ redo:
break; break;
} }
ret += chars; ret += chars;
buf->offset += chars;
buf->len -= chars; buf->len -= chars;
/* Was it a packet buffer? Clean up and exit */ /* Was it a packet buffer? Clean up and exit */
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment