Message ID | 1435612136-6680-1-git-send-email-kamal@canonical.com |
---|---|
State | New |
Headers | show |
On Mon, Jun 29, 2015 at 02:08:56PM -0700, Kamal Mostafa wrote: > From: Ben Hutchings <ben@decadent.org.uk> > > pipe_iov_copy_{from,to}_user() may be tried twice with the same iovec, > the first time atomically and the second time not. The second attempt > needs to continue from the iovec position, pipe buffer offset and > remaining length where the first attempt failed, but currently the > pipe buffer offset and remaining length are reset. This will corrupt > the piped data (possibly also leading to an information leak between > processes) and may also corrupt kernel memory. > > This was fixed upstream by commits f0d1bec9d58d ("new helper: > copy_page_from_iter()") and 637b58c2887e ("switch pipe_read() to > copy_page_to_iter()"), but those aren't suitable for stable. This fix > for older kernel versions was made by Seth Jennings for RHEL and I > have extracted it from their update. > > CVE-2015-1805 > > References: https://bugzilla.redhat.com/show_bug.cgi?id=1202855 > Cc: stable <stable@vger.kernel.org> # 3.14 and earlier > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > --- > fs/pipe.c | 55 ++++++++++++++++++++++++++++++++----------------------- > 1 file changed, 32 insertions(+), 23 deletions(-) > > diff --git a/fs/pipe.c b/fs/pipe.c > index 8ca88fc..d2cbeff 100644 > --- a/fs/pipe.c > +++ b/fs/pipe.c > @@ -103,25 +103,27 @@ void pipe_wait(struct pipe_inode_info *pipe) > } > > static int > -pipe_iov_copy_from_user(void *to, struct iovec *iov, unsigned long len, > - int atomic) > +pipe_iov_copy_from_user(void *addr, int *offset, struct iovec *iov, > + size_t *remaining, int atomic) > { > unsigned long copy; > > - while (len > 0) { > + while (*remaining > 0) { > while (!iov->iov_len) > iov++; > - copy = min_t(unsigned long, len, iov->iov_len); > + copy = min_t(unsigned long, *remaining, iov->iov_len); > > if (atomic) { > - if (__copy_from_user_inatomic(to, iov->iov_base, copy)) > + if (__copy_from_user_inatomic(addr + *offset, > + iov->iov_base, copy)) > return -EFAULT; > } else { > - if (copy_from_user(to, iov->iov_base, copy)) > + if (copy_from_user(addr + *offset, > + iov->iov_base, copy)) > return -EFAULT; > } > - to += copy; > - len -= copy; > + *offset += copy; > + *remaining -= copy; > iov->iov_base += copy; > iov->iov_len -= copy; > } > @@ -129,25 +131,27 @@ pipe_iov_copy_from_user(void *to, struct iovec *iov, unsigned long len, > } > > static int > -pipe_iov_copy_to_user(struct iovec *iov, const void *from, unsigned long len, > - int atomic) > +pipe_iov_copy_to_user(struct iovec *iov, void *addr, int *offset, > + size_t *remaining, int atomic) > { > unsigned long copy; > > - while (len > 0) { > + while (*remaining > 0) { > while (!iov->iov_len) > iov++; > - copy = min_t(unsigned long, len, iov->iov_len); > + copy = min_t(unsigned long, *remaining, iov->iov_len); > > if (atomic) { > - if (__copy_to_user_inatomic(iov->iov_base, from, copy)) > + if (__copy_to_user_inatomic(iov->iov_base, > + addr + *offset, copy)) > return -EFAULT; > } else { > - if (copy_to_user(iov->iov_base, from, copy)) > + if (copy_to_user(iov->iov_base, > + addr + *offset, copy)) > return -EFAULT; > } > - from += copy; > - len -= copy; > + *offset += copy; > + *remaining -= copy; > iov->iov_base += copy; > iov->iov_len -= copy; > } > @@ -383,7 +387,7 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov, > struct pipe_buffer *buf = pipe->bufs + curbuf; > const struct pipe_buf_operations *ops = buf->ops; > void *addr; > - size_t chars = buf->len; > + size_t chars = buf->len, remaining; > int error, atomic; > > if (chars > total_len) > @@ -397,9 +401,11 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov, > } > > atomic = !iov_fault_in_pages_write(iov, chars); > + remaining = chars; > redo: > addr = ops->map(pipe, buf, atomic); > - error = pipe_iov_copy_to_user(iov, addr + buf->offset, chars, atomic); > + error = pipe_iov_copy_to_user(iov, addr, &buf->offset, > + &remaining, atomic); > ops->unmap(pipe, buf, addr); > if (unlikely(error)) { > /* > @@ -414,7 +420,6 @@ redo: > break; > } > ret += chars; > - buf->offset += chars; > buf->len -= chars; > > /* Was it a packet buffer? Clean up and exit */ > @@ -521,6 +526,7 @@ pipe_write(struct kiocb *iocb, const struct iovec *_iov, > if (ops->can_merge && offset + chars <= PAGE_SIZE) { > int error, atomic = 1; > void *addr; > + size_t remaining = chars; > > error = ops->confirm(pipe, buf); > if (error) > @@ -529,8 +535,8 @@ pipe_write(struct kiocb *iocb, const struct iovec *_iov, > iov_fault_in_pages_read(iov, chars); > redo1: > addr = ops->map(pipe, buf, atomic); > - error = pipe_iov_copy_from_user(offset + addr, iov, > - chars, atomic); > + error = pipe_iov_copy_from_user(addr, &offset, iov, > + &remaining, atomic); > ops->unmap(pipe, buf, addr); > ret = error; > do_wakeup = 1; > @@ -565,6 +571,8 @@ redo1: > struct page *page = pipe->tmp_page; > char *src; > int error, atomic = 1; > + int offset = 0; > + size_t remaining; > > if (!page) { > page = alloc_page(GFP_HIGHUSER); > @@ -585,14 +593,15 @@ redo1: > chars = total_len; > > iov_fault_in_pages_read(iov, chars); > + remaining = chars; > redo2: > if (atomic) > src = kmap_atomic(page, KM_USER0); > else > src = kmap(page); > > - error = pipe_iov_copy_from_user(src, iov, chars, > - atomic); > + error = pipe_iov_copy_from_user(src, &offset, iov, > + &remaining, atomic); > if (atomic) > kunmap_atomic(src, KM_USER0); > else Seems to track offset,remaining reasonably. Acked-by: Andy Whitcroft <apw@canonical.com> -apw
diff --git a/fs/pipe.c b/fs/pipe.c index 8ca88fc..d2cbeff 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -103,25 +103,27 @@ void pipe_wait(struct pipe_inode_info *pipe) } static int -pipe_iov_copy_from_user(void *to, struct iovec *iov, unsigned long len, - int atomic) +pipe_iov_copy_from_user(void *addr, int *offset, struct iovec *iov, + size_t *remaining, int atomic) { unsigned long copy; - while (len > 0) { + while (*remaining > 0) { while (!iov->iov_len) iov++; - copy = min_t(unsigned long, len, iov->iov_len); + copy = min_t(unsigned long, *remaining, iov->iov_len); if (atomic) { - if (__copy_from_user_inatomic(to, iov->iov_base, copy)) + if (__copy_from_user_inatomic(addr + *offset, + iov->iov_base, copy)) return -EFAULT; } else { - if (copy_from_user(to, iov->iov_base, copy)) + if (copy_from_user(addr + *offset, + iov->iov_base, copy)) return -EFAULT; } - to += copy; - len -= copy; + *offset += copy; + *remaining -= copy; iov->iov_base += copy; iov->iov_len -= copy; } @@ -129,25 +131,27 @@ pipe_iov_copy_from_user(void *to, struct iovec *iov, unsigned long len, } static int -pipe_iov_copy_to_user(struct iovec *iov, const void *from, unsigned long len, - int atomic) +pipe_iov_copy_to_user(struct iovec *iov, void *addr, int *offset, + size_t *remaining, int atomic) { unsigned long copy; - while (len > 0) { + while (*remaining > 0) { while (!iov->iov_len) iov++; - copy = min_t(unsigned long, len, iov->iov_len); + copy = min_t(unsigned long, *remaining, iov->iov_len); if (atomic) { - if (__copy_to_user_inatomic(iov->iov_base, from, copy)) + if (__copy_to_user_inatomic(iov->iov_base, + addr + *offset, copy)) return -EFAULT; } else { - if (copy_to_user(iov->iov_base, from, copy)) + if (copy_to_user(iov->iov_base, + addr + *offset, copy)) return -EFAULT; } - from += copy; - len -= copy; + *offset += copy; + *remaining -= copy; iov->iov_base += copy; iov->iov_len -= copy; } @@ -383,7 +387,7 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov, struct pipe_buffer *buf = pipe->bufs + curbuf; const struct pipe_buf_operations *ops = buf->ops; void *addr; - size_t chars = buf->len; + size_t chars = buf->len, remaining; int error, atomic; if (chars > total_len) @@ -397,9 +401,11 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov, } atomic = !iov_fault_in_pages_write(iov, chars); + remaining = chars; redo: addr = ops->map(pipe, buf, atomic); - error = pipe_iov_copy_to_user(iov, addr + buf->offset, chars, atomic); + error = pipe_iov_copy_to_user(iov, addr, &buf->offset, + &remaining, atomic); ops->unmap(pipe, buf, addr); if (unlikely(error)) { /* @@ -414,7 +420,6 @@ redo: break; } ret += chars; - buf->offset += chars; buf->len -= chars; /* Was it a packet buffer? Clean up and exit */ @@ -521,6 +526,7 @@ pipe_write(struct kiocb *iocb, const struct iovec *_iov, if (ops->can_merge && offset + chars <= PAGE_SIZE) { int error, atomic = 1; void *addr; + size_t remaining = chars; error = ops->confirm(pipe, buf); if (error) @@ -529,8 +535,8 @@ pipe_write(struct kiocb *iocb, const struct iovec *_iov, iov_fault_in_pages_read(iov, chars); redo1: addr = ops->map(pipe, buf, atomic); - error = pipe_iov_copy_from_user(offset + addr, iov, - chars, atomic); + error = pipe_iov_copy_from_user(addr, &offset, iov, + &remaining, atomic); ops->unmap(pipe, buf, addr); ret = error; do_wakeup = 1; @@ -565,6 +571,8 @@ redo1: struct page *page = pipe->tmp_page; char *src; int error, atomic = 1; + int offset = 0; + size_t remaining; if (!page) { page = alloc_page(GFP_HIGHUSER); @@ -585,14 +593,15 @@ redo1: chars = total_len; iov_fault_in_pages_read(iov, chars); + remaining = chars; redo2: if (atomic) src = kmap_atomic(page, KM_USER0); else src = kmap(page); - error = pipe_iov_copy_from_user(src, iov, chars, - atomic); + error = pipe_iov_copy_from_user(src, &offset, iov, + &remaining, atomic); if (atomic) kunmap_atomic(src, KM_USER0); else