bonding: validate ip header before check IPPROTO_IGMP [Linux 4.4.187]

This Linux kernel change "bonding: validate ip header before check IPPROTO_IGMP" is included in the Linux 4.4.187 release. This change is authored by Cong Wang <xiyou.wangcong [at] gmail.com> on Mon Jul 1 20:40:24 2019 -0700. The commit for this change in Linux stable tree is 2eb5ddc (patch) which is from upstream commit 9d1bc24. The same Linux upstream change may have been applied to various maintained Linux releases and you can find all Linux releases containing changes from upstream 9d1bc24.

bonding: validate ip header before check IPPROTO_IGMP

[ Upstream commit 9d1bc24b52fb8c5d859f9a47084bf1179470e04c ]

bond_xmit_roundrobin() checks for IGMP packets but it parses
the IP header even before checking skb->protocol.

We should validate the IP header with pskb_may_pull() before
using iph->protocol.

Reported-and-tested-by: syzbot+e5be16aa39ad6e755391@syzkaller.appspotmail.com
Fixes: a2fd940f4cff ("bonding: fix broken multicast with round-robin mode")
Cc: Jay Vosburgh <j.vosburgh@gmail.com>
Cc: Veaceslav Falico <vfalico@gmail.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

There are 37 lines of Linux source code added/deleted in this change. Code changes to Linux kernel are as follows.

 drivers/net/bonding/bond_main.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0397afc..ed80c97 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3714,8 +3714,8 @@ static u32 bond_rr_gen_slave_id(struct bonding *bond)
 static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev)
 {
    struct bonding *bond = netdev_priv(bond_dev);
-   struct iphdr *iph = ip_hdr(skb);
    struct slave *slave;
+   int slave_cnt;
    u32 slave_id;

    /* Start with the curr_active_slave that joined the bond as the
@@ -3724,23 +3724,32 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev
     * send the join/membership reports.  The curr_active_slave found
     * will send all of this type of traffic.
     */
-   if (iph->protocol == IPPROTO_IGMP && skb->protocol == htons(ETH_P_IP)) {
-       slave = rcu_dereference(bond->curr_active_slave);
-       if (slave)
-           bond_dev_queue_xmit(bond, skb, slave->dev);
-       else
-           bond_xmit_slave_id(bond, skb, 0);
-   } else {
-       int slave_cnt = ACCESS_ONCE(bond->slave_cnt);
+   if (skb->protocol == htons(ETH_P_IP)) {
+       int noff = skb_network_offset(skb);
+       struct iphdr *iph;

-       if (likely(slave_cnt)) {
-           slave_id = bond_rr_gen_slave_id(bond);
-           bond_xmit_slave_id(bond, skb, slave_id % slave_cnt);
-       } else {
-           bond_tx_drop(bond_dev, skb);
+       if (unlikely(!pskb_may_pull(skb, noff + sizeof(*iph))))
+           goto non_igmp;
+
+       iph = ip_hdr(skb);
+       if (iph->protocol == IPPROTO_IGMP) {
+           slave = rcu_dereference(bond->curr_active_slave);
+           if (slave)
+               bond_dev_queue_xmit(bond, skb, slave->dev);
+           else
+               bond_xmit_slave_id(bond, skb, 0);
+           return NETDEV_TX_OK;
        }
    }

+non_igmp:
+   slave_cnt = ACCESS_ONCE(bond->slave_cnt);
+   if (likely(slave_cnt)) {
+       slave_id = bond_rr_gen_slave_id(bond);
+       bond_xmit_slave_id(bond, skb, slave_id % slave_cnt);
+   } else {
+       bond_tx_drop(bond_dev, skb);
+   }
    return NETDEV_TX_OK;
 }

Leave a Reply

Your email address will not be published. Required fields are marked *