Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 047fe360 authored by Eric Dumazet's avatar Eric Dumazet Committed by Jens Axboe
Browse files

splice: fix racy pipe->buffers uses 



Dave Jones reported a kernel BUG at mm/slub.c:3474! triggered
by splice_shrink_spd() called from vmsplice_to_pipe()

commit 35f3d14d (pipe: add support for shrinking and growing pipes)
added capability to adjust pipe->buffers.

Problem is some paths don't hold pipe mutex and assume pipe->buffers
doesn't change for their duration.

Fix this by adding nr_pages_max field in struct splice_pipe_desc, and
use it in place of pipe->buffers where appropriate.

splice_shrink_spd() loses its struct pipe_inode_info argument.

Reported-by: default avatarDave Jones <davej@redhat.com>
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Tom Herbert <therbert@google.com>
Cc: stable <stable@vger.kernel.org> # 2.6.35
Tested-by: default avatarDave Jones <davej@redhat.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 27e1f9d1

fs/splice.c

+20 −15
Original line number Diff line number Diff line
@@ -273,13 +273,16 @@ void spd_release_page(struct splice_pipe_desc *spd, unsigned int i) 
 * Check if we need to grow the arrays holding pages and partial page

 * descriptions.

 */

int splice_grow_spd(struct pipe_inode_info *pipe, struct splice_pipe_desc *spd)

int splice_grow_spd(const struct pipe_inode_info *pipe, struct splice_pipe_desc *spd)

{

	if (pipe->buffers <= PIPE_DEF_BUFFERS)

	unsigned int buffers = ACCESS_ONCE(pipe->buffers);



	spd->nr_pages_max = buffers;

	if (buffers <= PIPE_DEF_BUFFERS)

		return 0;



	spd->pages = kmalloc(pipe->buffers * sizeof(struct page *), GFP_KERNEL);

	spd->partial = kmalloc(pipe->buffers * sizeof(struct partial_page), GFP_KERNEL);

	spd->pages = kmalloc(buffers * sizeof(struct page *), GFP_KERNEL);

	spd->partial = kmalloc(buffers * sizeof(struct partial_page), GFP_KERNEL);



	if (spd->pages && spd->partial)

		return 0;
@@ -289,10 +292,9 @@ int splice_grow_spd(struct pipe_inode_info *pipe, struct splice_pipe_desc *spd) 
	return -ENOMEM;

}



void splice_shrink_spd(struct pipe_inode_info *pipe,

		       struct splice_pipe_desc *spd)

void splice_shrink_spd(struct splice_pipe_desc *spd)

{

	if (pipe->buffers <= PIPE_DEF_BUFFERS)

	if (spd->nr_pages_max <= PIPE_DEF_BUFFERS)

		return;



	kfree(spd->pages);
@@ -315,6 +317,7 @@ __generic_file_splice_read(struct file *in, loff_t *ppos, 
	struct splice_pipe_desc spd = {

		.pages = pages,

		.partial = partial,

		.nr_pages_max = PIPE_DEF_BUFFERS,

		.flags = flags,

		.ops = &page_cache_pipe_buf_ops,

		.spd_release = spd_release_page,
@@ -326,7 +329,7 @@ __generic_file_splice_read(struct file *in, loff_t *ppos, 
	index = *ppos >> PAGE_CACHE_SHIFT;

	loff = *ppos & ~PAGE_CACHE_MASK;

	req_pages = (len + loff + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;

	nr_pages = min(req_pages, pipe->buffers);

	nr_pages = min(req_pages, spd.nr_pages_max);



	/*

	 * Lookup the (hopefully) full range of pages we need.
@@ -497,7 +500,7 @@ fill_it: 
	if (spd.nr_pages)

		error = splice_to_pipe(pipe, &spd);



	splice_shrink_spd(pipe, &spd);

	splice_shrink_spd(&spd);

	return error;

}
@@ -598,6 +601,7 @@ ssize_t default_file_splice_read(struct file *in, loff_t *ppos, 
	struct splice_pipe_desc spd = {

		.pages = pages,

		.partial = partial,

		.nr_pages_max = PIPE_DEF_BUFFERS,

		.flags = flags,

		.ops = &default_pipe_buf_ops,

		.spd_release = spd_release_page,
@@ -608,8 +612,8 @@ ssize_t default_file_splice_read(struct file *in, loff_t *ppos, 


	res = -ENOMEM;

	vec = __vec;

	if (pipe->buffers > PIPE_DEF_BUFFERS) {

		vec = kmalloc(pipe->buffers * sizeof(struct iovec), GFP_KERNEL);

	if (spd.nr_pages_max > PIPE_DEF_BUFFERS) {

		vec = kmalloc(spd.nr_pages_max * sizeof(struct iovec), GFP_KERNEL);

		if (!vec)

			goto shrink_ret;

	}
@@ -617,7 +621,7 @@ ssize_t default_file_splice_read(struct file *in, loff_t *ppos, 
	offset = *ppos & ~PAGE_CACHE_MASK;

	nr_pages = (len + offset + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;



	for (i = 0; i < nr_pages && i < pipe->buffers && len; i++) {

	for (i = 0; i < nr_pages && i < spd.nr_pages_max && len; i++) {

		struct page *page;



		page = alloc_page(GFP_USER);
@@ -665,7 +669,7 @@ ssize_t default_file_splice_read(struct file *in, loff_t *ppos, 
shrink_ret:

	if (vec != __vec)

		kfree(vec);

	splice_shrink_spd(pipe, &spd);

	splice_shrink_spd(&spd);

	return res;



err:
@@ -1614,6 +1618,7 @@ static long vmsplice_to_pipe(struct file *file, const struct iovec __user *iov, 
	struct splice_pipe_desc spd = {

		.pages = pages,

		.partial = partial,

		.nr_pages_max = PIPE_DEF_BUFFERS,

		.flags = flags,

		.ops = &user_page_pipe_buf_ops,

		.spd_release = spd_release_page,
@@ -1629,13 +1634,13 @@ static long vmsplice_to_pipe(struct file *file, const struct iovec __user *iov, 


	spd.nr_pages = get_iovec_page_array(iov, nr_segs, spd.pages,

					    spd.partial, false,

					    pipe->buffers);

					    spd.nr_pages_max);

	if (spd.nr_pages <= 0)

		ret = spd.nr_pages;

	else

		ret = splice_to_pipe(pipe, &spd);



	splice_shrink_spd(pipe, &spd);

	splice_shrink_spd(&spd);

	return ret;

}

include/linux/splice.h

+4 −4
Original line number Diff line number Diff line
@@ -51,7 +51,8 @@ struct partial_page { 
struct splice_pipe_desc {

	struct page **pages;		/* page map */

	struct partial_page *partial;	/* pages[] may not be contig */

	int nr_pages;			/* number of pages in map */

	int nr_pages;			/* number of populated pages in map */

	unsigned int nr_pages_max;	/* pages[] & partial[] arrays size */

	unsigned int flags;		/* splice flags */

	const struct pipe_buf_operations *ops;/* ops associated with output pipe */

	void (*spd_release)(struct splice_pipe_desc *, unsigned int);
@@ -85,9 +86,8 @@ extern ssize_t splice_direct_to_actor(struct file *, struct splice_desc *, 
/*

 * for dynamic pipe sizing

 */

extern int splice_grow_spd(struct pipe_inode_info *, struct splice_pipe_desc *);

extern void splice_shrink_spd(struct pipe_inode_info *,

				struct splice_pipe_desc *);

extern int splice_grow_spd(const struct pipe_inode_info *, struct splice_pipe_desc *);

extern void splice_shrink_spd(struct splice_pipe_desc *);

extern void spd_release_page(struct splice_pipe_desc *, unsigned int);



extern const struct pipe_buf_operations page_cache_pipe_buf_ops;

kernel/relay.c

+3 −2
Original line number Diff line number Diff line
@@ -1235,6 +1235,7 @@ static ssize_t subbuf_splice_actor(struct file *in, 
	struct splice_pipe_desc spd = {

		.pages = pages,

		.nr_pages = 0,

		.nr_pages_max = PIPE_DEF_BUFFERS,

		.partial = partial,

		.flags = flags,

		.ops = &relay_pipe_buf_ops,
@@ -1302,7 +1303,7 @@ static ssize_t subbuf_splice_actor(struct file *in, 
                ret += padding;



out:

	splice_shrink_spd(pipe, &spd);

	splice_shrink_spd(&spd);

	return ret;

}

kernel/trace/trace.c

+4 −2
Original line number Diff line number Diff line
@@ -3609,6 +3609,7 @@ static ssize_t tracing_splice_read_pipe(struct file *filp, 
		.pages		= pages_def,

		.partial	= partial_def,

		.nr_pages	= 0, /* This gets updated below. */

		.nr_pages_max	= PIPE_DEF_BUFFERS,

		.flags		= flags,

		.ops		= &tracing_pipe_buf_ops,

		.spd_release	= tracing_spd_release_pipe,
@@ -3680,7 +3681,7 @@ static ssize_t tracing_splice_read_pipe(struct file *filp, 


	ret = splice_to_pipe(pipe, &spd);

out:

	splice_shrink_spd(pipe, &spd);

	splice_shrink_spd(&spd);

	return ret;



out_err:
@@ -4231,6 +4232,7 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos, 
	struct splice_pipe_desc spd = {

		.pages		= pages_def,

		.partial	= partial_def,

		.nr_pages_max	= PIPE_DEF_BUFFERS,

		.flags		= flags,

		.ops		= &buffer_pipe_buf_ops,

		.spd_release	= buffer_spd_release,
@@ -4318,7 +4320,7 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos, 
	}



	ret = splice_to_pipe(pipe, &spd);

	splice_shrink_spd(pipe, &spd);

	splice_shrink_spd(&spd);

out:

	return ret;

}

mm/shmem.c

+2 −1
Original line number Diff line number Diff line
@@ -1577,6 +1577,7 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos, 
	struct splice_pipe_desc spd = {

		.pages = pages,

		.partial = partial,

		.nr_pages_max = PIPE_DEF_BUFFERS,

		.flags = flags,

		.ops = &page_cache_pipe_buf_ops,

		.spd_release = spd_release_page,
@@ -1665,7 +1666,7 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos, 
	if (spd.nr_pages)

		error = splice_to_pipe(pipe, &spd);



	splice_shrink_spd(pipe, &spd);

	splice_shrink_spd(&spd);



	if (error > 0) {

		*ppos += error;