Skip to content

Commit

Permalink
pipe: iovec: Fix memory corruption when retrying atomic copy as non-a…
Browse files Browse the repository at this point in the history
…tomic

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 f0d1bec ("new helper:
copy_page_from_iter()") and 637b58c ("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
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
[lizf: Backported to 3.4: adjust context]
Signed-off-by: Zefan Li <lizefan@huawei.com>
  • Loading branch information
Ben Hutchings authored and Zefan Li committed Sep 18, 2015
1 parent bee5f3e commit a39bf4a
Showing 1 changed file with 32 additions and 23 deletions.
55 changes: 32 additions & 23 deletions fs/pipe.c
Original file line number Diff line number Diff line change
@@ -104,51 +104,55 @@ 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;
}
return 0;
}

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;
}
@@ -384,7 +388,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)
@@ -398,9 +402,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)) {
/*
@@ -415,7 +421,6 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov,
break;
}
ret += chars;
buf->offset += chars;
buf->len -= chars;

/* Was it a packet buffer? Clean up and exit */
@@ -522,6 +527,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)
@@ -530,8 +536,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;
@@ -566,6 +572,8 @@ pipe_write(struct kiocb *iocb, const struct iovec *_iov,
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);
@@ -586,14 +594,15 @@ pipe_write(struct kiocb *iocb, const struct iovec *_iov,
chars = total_len;

iov_fault_in_pages_read(iov, chars);
remaining = chars;
redo2:
if (atomic)
src = kmap_atomic(page);
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);
else

0 comments on commit a39bf4a

Please sign in to comment.