This is a note to let you know that I've just added the patch titled

    bonding: Fix corrupted queue_mapping

to the 3.0-stable tree which can be found at:
    
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     bonding-fix-corrupted-queue_mapping.patch
and it can be found in the queue-3.0 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <[email protected]> know about it.


>From 572d9c7b6e91b84a2a122a8d30e47b3e834548ce Mon Sep 17 00:00:00 2001
From: Eric Dumazet <[email protected]>
Date: Tue, 12 Jun 2012 06:03:51 +0000
Subject: bonding: Fix corrupted queue_mapping


From: Eric Dumazet <[email protected]>

[ 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 <[email protected]>
Reported-by: Tom Herbert <[email protected]>
Cc: John Fastabend <[email protected]>
Cc: Roland Dreier <[email protected]>
Acked-by: Neil Horman <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
 drivers/net/bonding/bond_main.c |    9 +++++----
 include/net/sch_generic.h       |    7 +++++--
 2 files changed, 10 insertions(+), 6 deletions(-)

--- 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"
@@ -388,8 +389,6 @@ struct vlan_entry *bond_next_vlan(struct
        return next;
 }
 
-#define bond_queue_mapping(skb) (*(u16 *)((skb)->cb))
-
 /**
  * bond_dev_queue_xmit - Prepare skb for xmit.
  *
@@ -403,7 +402,9 @@ int bond_dev_queue_xmit(struct bonding *
        skb->dev = slave_dev;
        skb->priority = 1;
 
-       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);
@@ -4240,7 +4241,7 @@ static u16 bond_select_queue(struct net_
        /*
         * 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 {
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -219,13 +219,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);
 }
 


Patches currently in stable-queue which might be from [email protected] are

queue-3.0/be2net-fix-a-race-in-be_xmit.patch
queue-3.0/bonding-fix-corrupted-queue_mapping.patch
queue-3.0/netpoll-fix-netpoll_send_udp-bugs.patch
queue-3.0/dummy-fix-rcu_sched-self-detected-stalls.patch
queue-3.0/net-l2tp_eth-fix-kernel-panic-on-rmmod-l2tp_eth.patch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to