sky2: Fix crash on receiving VLAN frames
authorKirill Smelkov <kirr@mns.spb.ru>
Fri, 3 May 2013 04:22:04 +0000 (04:22 +0000)
committerDavid S. Miller <davem@davemloft.net>
Fri, 3 May 2013 20:10:34 +0000 (16:10 -0400)
After recent 86a9bad3 (net: vlan: add protocol argument to packet
tagging functions) my sky2 started to crash on receive of tagged
frames, with backtrace similar to

    #CRASH!!!
    vlan_do_receive
    __netif_receive_skb_core
    __netif_receive_skb
    netif_receive_skb
    sky2_poll
    ...
    __net_rx_action
    __do_softirq

The problem turned out to be:

    1) sky2 copies small packets from ring on RX, and in its
       receive_copy() skb header is copied manually field, by field, and
       only for some fields;

    2) 86a9bad3  added skb->vlan_proto, which vlan_untag() or
       __vlan_hwaccel_put_tag() set, and which is later used in
       vlan_do_receive().

       That patch updated copy_skb_header() for newly introduced
       skb->vlan_proto, but overlooked the need to also copy it in sky2's
       receive_copy().

Because of 2, we have the following scenario:

    - frame is received and tagged in a ring, by sky2_rx_tag(). Both
      skb->vlan_proto and skb->vlan_tci are set;

    - later skb is decided to be copied, but skb->vlan_proto is
      forgotten and becomes 0.

    - in the beginning of vlan_do_receive() we call

        __be16 vlan_proto = skb->vlan_proto;
        vlan_dev = vlan_find_dev(skb->dev, vlan_proto, vlan_id);

      which eventually invokes

        vlan_proto_idx(vlan_proto)

      and that routine BUGs for everything except ETH_P_8021Q and
      ETH_P_8021AD.

      Oops.

Fix it.

P.S.

Stephen, I wonder, why copy_skb_header() is not used in
sky2.c::receive_copy() ? Problems, where receive_copy was updated field
by field showed several times already, e.g.

    3f42941b    (sky2: propogate rx hash when packet is copied)
    e072b3fa    (sky2: fix receive length error in mixed non-VLAN/VLAN traffic)

Cc: Patrick McHardy <kaber@trash.net>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: Mirko Lindner <mlindner@marvell.com>
Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/ethernet/marvell/sky2.c

index 256ae789c1431c669835879b84587926d45a2d35..d175bbd3ffd37952f5dc2e66348049693ac7d2d7 100644 (file)
@@ -2496,10 +2496,12 @@ static struct sk_buff *receive_copy(struct sky2_port *sky2,
                skb->ip_summed = re->skb->ip_summed;
                skb->csum = re->skb->csum;
                skb->rxhash = re->skb->rxhash;
+               skb->vlan_proto = re->skb->vlan_proto;
                skb->vlan_tci = re->skb->vlan_tci;
 
                pci_dma_sync_single_for_device(sky2->hw->pdev, re->data_addr,
                                               length, PCI_DMA_FROMDEVICE);
+               re->skb->vlan_proto = 0;
                re->skb->vlan_tci = 0;
                re->skb->rxhash = 0;
                re->skb->ip_summed = CHECKSUM_NONE;