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>,
	Tom Herbert <therbert@google.com>,
	John Fastabend <john.r.fastabend@intel.com>,
	Roland Dreier <roland@kernel.org>,
	Neil Horman <nhorman@tuxdriver.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [ 36/48] bonding: Fix corrupted queue_mapping
Date: Mon, 09 Jul 2012 15:31:52 +0100	[thread overview]
Message-ID: <20120709143121.866486950@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>

[ Upstream commit 5ee31c6898ea5537fcea160999d60dc63bc0c305 ]

In the transmit path of the bonding driver, skb->cb is used to
stash the skb->queue_mapping so that the bonding device can set its
own queue mapping.  This value becomes corrupted since the skb->cb is
also used in __dev_xmit_skb.

When transmitting through bonding driver, bond_select_queue is
called from dev_queue_xmit.  In bond_select_queue the original
skb->queue_mapping is copied into skb->cb (via bond_queue_mapping)
and skb->queue_mapping is overwritten with the bond driver queue.

Subsequently in dev_queue_xmit, __dev_xmit_skb is called which writes
the packet length into skb->cb, thereby overwriting the stashed
queue mappping.  In bond_dev_queue_xmit (called from hard_start_xmit),
the queue mapping for the skb is set to the stashed value which is now
the skb length and hence is an invalid queue for the slave device.

If we want to save skb->queue_mapping into skb->cb[], best place is to
add a field in struct qdisc_skb_cb, to make sure it wont conflict with
other layers (eg : Qdiscc, Infiniband...)

This patchs also makes sure (struct qdisc_skb_cb)->data is aligned on 8
bytes :

netem qdisc for example assumes it can store an u64 in it, without
misalignment penalty.

Note : we only have 20 bytes left in (struct qdisc_skb_cb)->data[].
The largest user is CHOKe and it fills it.

Based on a previous patch from Tom Herbert.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Tom Herbert <therbert@google.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Roland Dreier <roland@kernel.org>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/net/bonding/bond_main.c |    9 +++++----
 include/net/sch_generic.h       |    7 +++++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f65e0b9..1a88e38 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -77,6 +77,7 @@
 #include <net/route.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
+#include <net/pkt_sched.h>
 #include "bonding.h"
 #include "bond_3ad.h"
 #include "bond_alb.h"
@@ -382,8 +383,6 @@ struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr)
 	return next;
 }
 
-#define bond_queue_mapping(skb) (*(u16 *)((skb)->cb))
-
 /**
  * bond_dev_queue_xmit - Prepare skb for xmit.
  *
@@ -396,7 +395,9 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
 {
 	skb->dev = slave_dev;
 
-	skb->queue_mapping = bond_queue_mapping(skb);
+	BUILD_BUG_ON(sizeof(skb->queue_mapping) !=
+		     sizeof(qdisc_skb_cb(skb)->bond_queue_mapping));
+	skb->queue_mapping = qdisc_skb_cb(skb)->bond_queue_mapping;
 
 	if (unlikely(netpoll_tx_running(slave_dev)))
 		bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
@@ -4151,7 +4152,7 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
 	/*
 	 * Save the original txq to restore before passing to the driver
 	 */
-	bond_queue_mapping(skb) = skb->queue_mapping;
+	qdisc_skb_cb(skb)->bond_queue_mapping = skb->queue_mapping;
 
 	if (unlikely(txq >= dev->real_num_tx_queues)) {
 		do {
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 55ce96b..9d7d54a 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -220,13 +220,16 @@ struct tcf_proto {
 
 struct qdisc_skb_cb {
 	unsigned int		pkt_len;
-	unsigned char		data[24];
+	u16			bond_queue_mapping;
+	u16			_pad;
+	unsigned char		data[20];
 };
 
 static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz)
 {
 	struct qdisc_skb_cb *qcb;
-	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(unsigned int) + sz);
+
+	BUILD_BUG_ON(sizeof(skb->cb) < offsetof(struct qdisc_skb_cb, data) + sz);
 	BUILD_BUG_ON(sizeof(qcb->data) < sz);
 }
 



  parent reply	other threads:[~2012-07-09 14:43 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 ` [ 01/48] splice: fix racy pipe->buffers uses Ben Hutchings
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 ` Ben Hutchings [this message]
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=20120709143121.866486950@decadent.org.uk \
    --to=ben@decadent.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=john.r.fastabend@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=roland@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=therbert@google.com \
    --cc=torvalds@linux-foundation.org \
    /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

Powered by JetHome