mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ben Hutchings <ben@decadent.org.uk>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org,
	alan@lxorguk.ukuu.org.uk, Eric Dumazet <edumazet@google.com>,
	Dave Jones <davej@redhat.com>, Jens Axboe <axboe@kernel.dk>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Tom Herbert <therbert@google.com>
Subject: [ 01/48] splice: fix racy pipe->buffers uses
Date: Mon, 09 Jul 2012 15:31:17 +0100	[thread overview]
Message-ID: <20120709143116.809234556@decadent.org.uk> (raw)
In-Reply-To: <20120709143116.348975190@decadent.org.uk>

3.2-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Eric Dumazet <edumazet@google.com>

commit 047fe3605235888f3ebcda0c728cb31937eadfe6 upstream.

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

commit 35f3d14dbbc5 (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: Dave Jones <davej@redhat.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Tom Herbert <therbert@google.com>
Tested-by: Dave Jones <davej@redhat.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
[bwh: Backported to 3.2:
 - Adjust context in vmsplice_to_pipe()
 - Update one more call to splice_shrink_spd(), from skb_splice_bits()]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 fs/splice.c            |   35 ++++++++++++++++++++---------------
 include/linux/splice.h |    8 ++++----
 kernel/relay.c         |    5 +++--
 kernel/trace/trace.c   |    6 ++++--
 mm/shmem.c             |    3 ++-
 net/core/skbuff.c      |    1 +
 6 files changed, 34 insertions(+), 24 deletions(-)

--- a/fs/splice.c
+++ b/fs/splice.c
@@ -274,13 +274,16 @@ void spd_release_page(struct splice_pipe
  * 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;
@@ -290,10 +293,9 @@ int splice_grow_spd(struct pipe_inode_in
 	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);
@@ -316,6 +318,7 @@ __generic_file_splice_read(struct file *
 	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,
@@ -327,7 +330,7 @@ __generic_file_splice_read(struct file *
 	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.
@@ -498,7 +501,7 @@ fill_it:
 	if (spd.nr_pages)
 		error = splice_to_pipe(pipe, &spd);
 
-	splice_shrink_spd(pipe, &spd);
+	splice_shrink_spd(&spd);
 	return error;
 }
 
@@ -599,6 +602,7 @@ ssize_t default_file_splice_read(struct
 	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,
@@ -609,8 +613,8 @@ ssize_t default_file_splice_read(struct
 
 	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;
 	}
@@ -618,7 +622,7 @@ ssize_t default_file_splice_read(struct
 	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);
@@ -666,7 +670,7 @@ ssize_t default_file_splice_read(struct
 shrink_ret:
 	if (vec != __vec)
 		kfree(vec);
-	splice_shrink_spd(pipe, &spd);
+	splice_shrink_spd(&spd);
 	return res;
 
 err:
@@ -1616,6 +1620,7 @@ static long vmsplice_to_pipe(struct file
 	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,
@@ -1631,13 +1636,13 @@ static long vmsplice_to_pipe(struct file
 
 	spd.nr_pages = get_iovec_page_array(iov, nr_segs, spd.pages,
 					    spd.partial, flags & SPLICE_F_GIFT,
-					    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;
 }
 
--- a/include/linux/splice.h
+++ b/include/linux/splice.h
@@ -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(st
 /*
  * 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;
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -1235,6 +1235,7 @@ static ssize_t subbuf_splice_actor(struc
 	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,8 +1303,8 @@ static ssize_t subbuf_splice_actor(struc
                 ret += padding;
 
 out:
-	splice_shrink_spd(pipe, &spd);
-        return ret;
+	splice_shrink_spd(&spd);
+	return ret;
 }
 
 static ssize_t relay_file_splice_read(struct file *in,
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3456,6 +3456,7 @@ static ssize_t tracing_splice_read_pipe(
 		.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,
@@ -3527,7 +3528,7 @@ static ssize_t tracing_splice_read_pipe(
 
 	ret = splice_to_pipe(pipe, &spd);
 out:
-	splice_shrink_spd(pipe, &spd);
+	splice_shrink_spd(&spd);
 	return ret;
 
 out_err:
@@ -4017,6 +4018,7 @@ tracing_buffers_splice_read(struct file
 	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,
@@ -4104,7 +4106,7 @@ tracing_buffers_splice_read(struct file
 	}
 
 	ret = splice_to_pipe(pipe, &spd);
-	splice_shrink_spd(pipe, &spd);
+	splice_shrink_spd(&spd);
 out:
 	return ret;
 }
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1359,6 +1359,7 @@ static ssize_t shmem_file_splice_read(st
 	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,
@@ -1447,7 +1448,7 @@ static ssize_t shmem_file_splice_read(st
 	if (spd.nr_pages)
 		error = splice_to_pipe(pipe, &spd);
 
-	splice_shrink_spd(pipe, &spd);
+	splice_shrink_spd(&spd);
 
 	if (error > 0) {
 		*ppos += error;
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1663,6 +1663,7 @@ int skb_splice_bits(struct sk_buff *skb,
 	struct splice_pipe_desc spd = {
 		.pages = pages,
 		.partial = partial,
+		.nr_pages_max = MAX_SKB_FRAGS,
 		.flags = flags,
 		.ops = &sock_pipe_buf_ops,
 		.spd_release = sock_spd_release,
@@ -1709,7 +1710,7 @@ done:
 		lock_sock(sk);
 	}
 
-	splice_shrink_spd(pipe, &spd);
+	splice_shrink_spd(&spd);
 	return ret;
 }
 



  reply	other threads:[~2012-07-09 14:37 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-09 14:31 [ 00/48] 3.2.23-stable review Ben Hutchings
2012-07-09 14:31 ` Ben Hutchings [this message]
2012-07-09 14:31 ` [ 02/48] umem: fix up unplugging Ben Hutchings
2012-07-09 14:31 ` [ 03/48] mwifiex: fix 11n rx packet drop issue Ben Hutchings
2012-07-09 14:31 ` [ 04/48] mwifiex: fix WPS eapol handshake failure Ben Hutchings
2012-07-09 14:31 ` [ 05/48] NFC: Prevent multiple buffer overflows in NCI Ben Hutchings
2012-07-09 14:31 ` [ 06/48] ath9k: fix dynamic WEP related regression Ben Hutchings
2012-07-09 14:31 ` [ 07/48] NFC: Return from rawsock_release when sk is NULL Ben Hutchings
2012-07-09 14:31 ` [ 08/48] rtlwifi: rtl8192cu: New USB IDs Ben Hutchings
2012-07-09 14:31 ` [ 09/48] ath9k: enable serialize_regmode for non-PCIE AR9287 Ben Hutchings
2012-07-09 14:31 ` [ 10/48] mac80211: correct behaviour on unrecognised action frames Ben Hutchings
2012-07-09 14:31 ` [ 11/48] ASoC: tlv320aic3x: Fix codec pll configure bug Ben Hutchings
2012-07-09 14:31 ` [ 12/48] powerpc/xmon: Use cpumask iterator to avoid warning Ben Hutchings
2012-07-09 14:31 ` [ 13/48] powerpc/kvm: sldi should be sld Ben Hutchings
2012-07-09 14:31 ` [ 14/48] md/raid10: Dont try to recovery unmatched (and unused) chunks Ben Hutchings
2012-07-09 14:31 ` [ 15/48] md/raid5: Do not add data_offset before call to is_badblock Ben Hutchings
2012-07-09 14:31 ` [ 16/48] md/raid5: In ops_run_io, inc nr_pending before calling md_wait_for_blocked_rdev Ben Hutchings
2012-07-09 14:31 ` [ 17/48] md/raid10: fix failure when trying to repair a read error Ben Hutchings
2012-07-09 14:31 ` [ 18/48] drm/i915: kick any firmware framebuffers before claiming the gtt Ben Hutchings
2012-07-09 14:31 ` [ 19/48] dm persistent data: fix shadow_info_leak on dm_tm_destroy Ben Hutchings
2012-07-09 14:31 ` [ 20/48] dm persistent data: handle space map checker creation failure Ben Hutchings
2012-07-09 14:31 ` [ 21/48] dm persistent data: fix allocation failure in space map checker init Ben Hutchings
2012-07-09 14:31 ` [ 22/48] ALSA: hda - Fix power-map regression for HP dv6 & co Ben Hutchings
2012-07-09 14:31 ` [ 23/48] tracing: change CPU ring buffer state from tracing_cpumask Ben Hutchings
2012-07-09 14:31 ` [ 24/48] mwifiex: fix wrong return values in add_virtual_intf() error cases Ben Hutchings
2012-07-09 14:31 ` [ 25/48] udf: Use ret instead of abusing i in udf_load_logicalvol() Ben Hutchings
2012-07-09 14:31 ` [ 26/48] udf: Avoid run away loop when partition table length is corrupted Ben Hutchings
2012-07-10 15:52   ` Ben Hutchings
2012-07-10 16:04     ` Jan Kara
2012-07-11  1:36       ` Ben Hutchings
2012-07-09 14:31 ` [ 27/48] udf: Fortify loading of sparing table Ben Hutchings
2012-07-09 14:31 ` [ 28/48] ARM: fix rcu stalls on SMP platforms Ben Hutchings
2012-07-09 14:31 ` [ 29/48] net: sock: validate data_len before allocating skb in sock_alloc_send_pskb() Ben Hutchings
2012-07-09 14:31 ` [ 30/48] cipso: handle CIPSO options correctly when NetLabel is disabled Ben Hutchings
2012-07-09 14:31 ` [ 31/48] net: l2tp_eth: fix kernel panic on rmmod l2tp_eth Ben Hutchings
2012-07-09 14:31 ` [ 32/48] l2tp: fix a race in l2tp_ip_sendmsg() Ben Hutchings
2012-07-09 14:31 ` [ 33/48] sky2: fix checksum bit management on some chips Ben Hutchings
2012-07-09 14:31 ` [ 34/48] be2net: fix a race in be_xmit() Ben Hutchings
2012-07-09 14:31 ` [ 35/48] dummy: fix rcu_sched self-detected stalls Ben Hutchings
2012-07-09 23:39   ` Herton Ronaldo Krzesinski
2012-07-09 23:47     ` Ben Hutchings
2012-07-09 23:49       ` David Miller
2012-07-10  0:20         ` Ben Hutchings
2012-07-09 14:31 ` [ 36/48] bonding: Fix corrupted queue_mapping Ben Hutchings
2012-07-09 14:31 ` [ 37/48] ethtool: allow ETHTOOL_GSSET_INFO for users Ben Hutchings
2012-07-09 14:31 ` [ 38/48] netpoll: fix netpoll_send_udp() bugs Ben Hutchings
2012-07-09 14:31 ` [ 39/48] ipv6: Move ipv6 proc file registration to end of init order Ben Hutchings
2012-07-09 14:31 ` [ 40/48] bridge: Assign rtnl_link_ops to bridge devices created via ioctl (v2) Ben Hutchings
2012-07-09 14:31 ` [ 41/48] Btrfs: run delayed directory updates during log replay Ben Hutchings
2012-07-09 14:31 ` [ 42/48] cifs: when server doesnt set CAP_LARGE_READ_X, cap default rsize at MaxBufferSize Ben Hutchings
2012-07-09 14:31 ` [ 43/48] ocfs2: clear unaligned io flag when dio fails Ben Hutchings
2012-07-09 14:32 ` [ 44/48] aio: make kiocb->private NUll in init_sync_kiocb() Ben Hutchings
2012-07-09 14:32 ` [ 45/48] mtd: cafe_nand: fix an & vs | mistake Ben Hutchings
2012-07-09 14:32 ` [ 46/48] mm: Hold a file reference in madvise_remove Ben Hutchings
2012-07-09 14:32 ` [ 47/48] tcm_fc: Resolve suspicious RCU usage warnings Ben Hutchings
2012-07-09 14:32 ` [ 48/48] vfs: make O_PATH file descriptors usable for fchdir() Ben Hutchings
2012-07-11  1:33 ` [ 00/48] 3.2.23-stable review Ben Hutchings

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120709143116.809234556@decadent.org.uk \
    --to=ben@decadent.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=axboe@kernel.dk \
    --cc=davej@redhat.com \
    --cc=edumazet@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=therbert@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®