Commit a2394a66 authored by Matthew Wilcox's avatar Matthew Wilcox Committed by Stefan Bader

fs: prevent page refcount overflow in pipe_buf_get

CVE-2019-11487

Change pipe_buf_get() to return a bool indicating whether it succeeded
in raising the refcount of the page (if the thing in the pipe is a page).
This removes another mechanism for overflowing the page refcount.  All
callers converted to handle a failure.
Reported-by: default avatarJann Horn <jannh@google.com>
Signed-off-by: default avatarMatthew Wilcox <willy@infradead.org>
Cc: stable@kernel.org
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
(backported from commit 15fab63e)
[ Connor Kuehl: required slight offset adjustments for the out_free
  label and changing the function signature for buffer_pipe_buf_get ]
Signed-off-by: default avatarConnor Kuehl <connor.kuehl@canonical.com>
Acked-by: default avatarTyler Hicks <tyhicks@canonical.com>
Acked-by: default avatarStefan Bader <stefan.bader@canonical.com>
Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
parent 3596fb0f
...@@ -2044,10 +2044,8 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, ...@@ -2044,10 +2044,8 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe,
rem += pipe->bufs[(pipe->curbuf + idx) & (pipe->buffers - 1)].len; rem += pipe->bufs[(pipe->curbuf + idx) & (pipe->buffers - 1)].len;
ret = -EINVAL; ret = -EINVAL;
if (rem < len) { if (rem < len)
pipe_unlock(pipe); goto out_free;
goto out;
}
rem = len; rem = len;
while (rem) { while (rem) {
...@@ -2065,7 +2063,9 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, ...@@ -2065,7 +2063,9 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe,
pipe->curbuf = (pipe->curbuf + 1) & (pipe->buffers - 1); pipe->curbuf = (pipe->curbuf + 1) & (pipe->buffers - 1);
pipe->nrbufs--; pipe->nrbufs--;
} else { } else {
pipe_buf_get(pipe, ibuf); if (!pipe_buf_get(pipe, ibuf))
goto out_free;
*obuf = *ibuf; *obuf = *ibuf;
obuf->flags &= ~PIPE_BUF_FLAG_GIFT; obuf->flags &= ~PIPE_BUF_FLAG_GIFT;
obuf->len = rem; obuf->len = rem;
...@@ -2088,13 +2088,13 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, ...@@ -2088,13 +2088,13 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe,
ret = fuse_dev_do_write(fud, &cs, len); ret = fuse_dev_do_write(fud, &cs, len);
pipe_lock(pipe); pipe_lock(pipe);
out_free:
for (idx = 0; idx < nbuf; idx++) { for (idx = 0; idx < nbuf; idx++) {
struct pipe_buffer *buf = &bufs[idx]; struct pipe_buffer *buf = &bufs[idx];
buf->ops->release(pipe, buf); buf->ops->release(pipe, buf);
} }
pipe_unlock(pipe); pipe_unlock(pipe);
out:
kfree(bufs); kfree(bufs);
return ret; return ret;
} }
......
...@@ -178,9 +178,9 @@ EXPORT_SYMBOL(generic_pipe_buf_steal); ...@@ -178,9 +178,9 @@ EXPORT_SYMBOL(generic_pipe_buf_steal);
* in the tee() system call, when we duplicate the buffers in one * in the tee() system call, when we duplicate the buffers in one
* pipe into another. * pipe into another.
*/ */
void generic_pipe_buf_get(struct pipe_inode_info *pipe, struct pipe_buffer *buf) bool generic_pipe_buf_get(struct pipe_inode_info *pipe, struct pipe_buffer *buf)
{ {
page_cache_get(buf->page); return try_get_page(buf->page);
} }
EXPORT_SYMBOL(generic_pipe_buf_get); EXPORT_SYMBOL(generic_pipe_buf_get);
......
...@@ -1878,7 +1878,11 @@ static int splice_pipe_to_pipe(struct pipe_inode_info *ipipe, ...@@ -1878,7 +1878,11 @@ static int splice_pipe_to_pipe(struct pipe_inode_info *ipipe,
* Get a reference to this pipe buffer, * Get a reference to this pipe buffer,
* so we can copy the contents over. * so we can copy the contents over.
*/ */
pipe_buf_get(ipipe, ibuf); if (!pipe_buf_get(ipipe, ibuf)) {
if (ret == 0)
ret = -EFAULT;
break;
}
*obuf = *ibuf; *obuf = *ibuf;
/* /*
...@@ -1950,7 +1954,11 @@ static int link_pipe(struct pipe_inode_info *ipipe, ...@@ -1950,7 +1954,11 @@ static int link_pipe(struct pipe_inode_info *ipipe,
* Get a reference to this pipe buffer, * Get a reference to this pipe buffer,
* so we can copy the contents over. * so we can copy the contents over.
*/ */
pipe_buf_get(ipipe, ibuf); if (!pipe_buf_get(ipipe, ibuf)) {
if (ret == 0)
ret = -EFAULT;
break;
}
obuf = opipe->bufs + nbuf; obuf = opipe->bufs + nbuf;
*obuf = *ibuf; *obuf = *ibuf;
......
...@@ -112,18 +112,20 @@ struct pipe_buf_operations { ...@@ -112,18 +112,20 @@ struct pipe_buf_operations {
/* /*
* Get a reference to the pipe buffer. * Get a reference to the pipe buffer.
*/ */
void (*get)(struct pipe_inode_info *, struct pipe_buffer *); bool (*get)(struct pipe_inode_info *, struct pipe_buffer *);
}; };
/** /**
* pipe_buf_get - get a reference to a pipe_buffer * pipe_buf_get - get a reference to a pipe_buffer
* @pipe: the pipe that the buffer belongs to * @pipe: the pipe that the buffer belongs to
* @buf: the buffer to get a reference to * @buf: the buffer to get a reference to
*
* Return: %true if the reference was successfully obtained.
*/ */
static inline void pipe_buf_get(struct pipe_inode_info *pipe, static inline __must_check bool pipe_buf_get(struct pipe_inode_info *pipe,
struct pipe_buffer *buf) struct pipe_buffer *buf)
{ {
buf->ops->get(pipe, buf); return buf->ops->get(pipe, buf);
} }
/* Differs from PIPE_BUF in that PIPE_SIZE is the length of the actual /* Differs from PIPE_BUF in that PIPE_SIZE is the length of the actual
...@@ -148,7 +150,7 @@ struct pipe_inode_info *alloc_pipe_info(void); ...@@ -148,7 +150,7 @@ struct pipe_inode_info *alloc_pipe_info(void);
void free_pipe_info(struct pipe_inode_info *); void free_pipe_info(struct pipe_inode_info *);
/* Generic pipe buffer ops functions */ /* Generic pipe buffer ops functions */
void generic_pipe_buf_get(struct pipe_inode_info *, struct pipe_buffer *); bool generic_pipe_buf_get(struct pipe_inode_info *, struct pipe_buffer *);
int generic_pipe_buf_confirm(struct pipe_inode_info *, struct pipe_buffer *); int generic_pipe_buf_confirm(struct pipe_inode_info *, struct pipe_buffer *);
int generic_pipe_buf_steal(struct pipe_inode_info *, struct pipe_buffer *); int generic_pipe_buf_steal(struct pipe_inode_info *, struct pipe_buffer *);
void generic_pipe_buf_release(struct pipe_inode_info *, struct pipe_buffer *); void generic_pipe_buf_release(struct pipe_inode_info *, struct pipe_buffer *);
......
...@@ -5735,12 +5735,16 @@ static void buffer_pipe_buf_release(struct pipe_inode_info *pipe, ...@@ -5735,12 +5735,16 @@ static void buffer_pipe_buf_release(struct pipe_inode_info *pipe,
buf->private = 0; buf->private = 0;
} }
static void buffer_pipe_buf_get(struct pipe_inode_info *pipe, static bool buffer_pipe_buf_get(struct pipe_inode_info *pipe,
struct pipe_buffer *buf) struct pipe_buffer *buf)
{ {
struct buffer_ref *ref = (struct buffer_ref *)buf->private; struct buffer_ref *ref = (struct buffer_ref *)buf->private;
if (ref->ref > INT_MAX/2)
return false;
ref->ref++; ref->ref++;
return true;
} }
/* Pipe buffer operations for a buffer. */ /* Pipe buffer operations for a buffer. */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment