lkml.org 
[lkml]   [2019]   [May]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 07/49] fs: prevent page refcount overflow in pipe_buf_get
    Date
    From: Matthew Wilcox <willy@infradead.org>

    commit 15fab63e1e57be9fdb5eec1bbc5916e9825e9acb upstream.

    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: Jann Horn <jannh@google.com>
    Signed-off-by: Matthew Wilcox <willy@infradead.org>
    Cc: stable@kernel.org
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/fuse/dev.c | 12 ++++++------
    fs/pipe.c | 4 ++--
    fs/splice.c | 12 ++++++++++--
    include/linux/pipe_fs_i.h | 10 ++++++----
    kernel/trace/trace.c | 6 +++++-
    5 files changed, 29 insertions(+), 15 deletions(-)

    --- a/fs/fuse/dev.c
    +++ b/fs/fuse/dev.c
    @@ -1981,10 +1981,8 @@ static ssize_t fuse_dev_splice_write(str
    rem += pipe->bufs[(pipe->curbuf + idx) & (pipe->buffers - 1)].len;

    ret = -EINVAL;
    - if (rem < len) {
    - pipe_unlock(pipe);
    - goto out;
    - }
    + if (rem < len)
    + goto out_free;

    rem = len;
    while (rem) {
    @@ -2002,7 +2000,9 @@ static ssize_t fuse_dev_splice_write(str
    pipe->curbuf = (pipe->curbuf + 1) & (pipe->buffers - 1);
    pipe->nrbufs--;
    } else {
    - pipe_buf_get(pipe, ibuf);
    + if (!pipe_buf_get(pipe, ibuf))
    + goto out_free;
    +
    *obuf = *ibuf;
    obuf->flags &= ~PIPE_BUF_FLAG_GIFT;
    obuf->len = rem;
    @@ -2025,11 +2025,11 @@ static ssize_t fuse_dev_splice_write(str
    ret = fuse_dev_do_write(fud, &cs, len);

    pipe_lock(pipe);
    +out_free:
    for (idx = 0; idx < nbuf; idx++)
    pipe_buf_release(pipe, &bufs[idx]);
    pipe_unlock(pipe);

    -out:
    kfree(bufs);
    return ret;
    }
    --- a/fs/pipe.c
    +++ b/fs/pipe.c
    @@ -194,9 +194,9 @@ EXPORT_SYMBOL(generic_pipe_buf_steal);
    * in the tee() system call, when we duplicate the buffers in one
    * 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)
    {
    - get_page(buf->page);
    + return try_get_page(buf->page);
    }
    EXPORT_SYMBOL(generic_pipe_buf_get);

    --- a/fs/splice.c
    +++ b/fs/splice.c
    @@ -1571,7 +1571,11 @@ retry:
    * Get a reference to this pipe buffer,
    * 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;

    /*
    @@ -1645,7 +1649,11 @@ static int link_pipe(struct pipe_inode_i
    * Get a reference to this pipe buffer,
    * 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 = *ibuf;
    --- a/include/linux/pipe_fs_i.h
    +++ b/include/linux/pipe_fs_i.h
    @@ -108,18 +108,20 @@ struct pipe_buf_operations {
    /*
    * 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: the pipe that the buffer belongs 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)
    {
    - buf->ops->get(pipe, buf);
    + return buf->ops->get(pipe, buf);
    }

    /**
    @@ -179,7 +181,7 @@ struct pipe_inode_info *alloc_pipe_info(
    void free_pipe_info(struct pipe_inode_info *);

    /* 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_steal(struct pipe_inode_info *, struct pipe_buffer *);
    int generic_pipe_buf_nosteal(struct pipe_inode_info *, struct pipe_buffer *);
    --- a/kernel/trace/trace.c
    +++ b/kernel/trace/trace.c
    @@ -6739,12 +6739,16 @@ static void buffer_pipe_buf_release(stru
    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 buffer_ref *ref = (struct buffer_ref *)buf->private;

    + if (refcount_read(&ref->refcount) > INT_MAX/2)
    + return false;
    +
    refcount_inc(&ref->refcount);
    + return true;
    }

    /* Pipe buffer operations for a buffer. */

    \
     
     \ /
      Last update: 2019-05-02 17:48    [W:4.074 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site