• Toshiaki Makita's avatar
    net: Fix vlan untag for bridge and vlan_dev with reorder_hdr off · 4bbb3e0e
    Toshiaki Makita authored
    When we have a bridge with vlan_filtering on and a vlan device on top of
    it, packets would be corrupted in skb_vlan_untag() called from
    br_dev_xmit().
    
    The problem sits in skb_reorder_vlan_header() used in skb_vlan_untag(),
    which makes use of skb->mac_len. In this function mac_len is meant for
    handling rx path with vlan devices with reorder_header disabled, but in
    tx path mac_len is typically 0 and cannot be used, which is the problem
    in this case.
    
    The current code even does not properly handle rx path (skb_vlan_untag()
    called from __netif_receive_skb_core()) with reorder_header off actually.
    
    In rx path single tag case, it works as follows:
    
    - Before skb_reorder_vlan_header()
    
     mac_header                                data
       v                                        v
       +-------------------+-------------+------+----
       |        ETH        |    VLAN     | ETH  |
       |       ADDRS       | TPID | TCI  | TYPE |
       +-------------------+-------------+------+----
       <-------- mac_len --------->
                           <------------->
                            to be removed
    
    - After skb_reorder_vlan_header()
    
                mac_header                     data
                     v                          v
                     +-------------------+------+----
                     |        ETH        | ETH  |
                     |       ADDRS       | TYPE |
                     +-------------------+------+----
                     <-------- mac_len --------->
    
    This is ok, but in rx double tag case, it corrupts packets:
    
    - Before skb_reorder_vlan_header()
    
     mac_header                                              data
       v                                                      v
       +-------------------+-------------+-------------+------+----
       |        ETH        |    VLAN     |    VLAN     | ETH  |
       |       ADDRS       | TPID | TCI  | TPID | TCI  | TYPE |
       +-------------------+-------------+-------------+------+----
       <--------------- mac_len ---------------->
                                         <------------->
                                        should be removed
                           <--------------------------->
                             actually will be removed
    
    - After skb_reorder_vlan_header()
    
                mac_header                                   data
                     v                                        v
                                   +-------------------+------+----
                                   |        ETH        | ETH  |
                                   |       ADDRS       | TYPE |
                                   +-------------------+------+----
                     <--------------- mac_len ---------------->
    
    So, two of vlan tags are both removed while only inner one should be
    removed and mac_header (and mac_len) is broken.
    
    skb_vlan_untag() is meant for removing the vlan header at (skb->data - 2),
    so use skb->data and skb->mac_header to calculate the right offset.
    Reported-by: default avatarBrandon Carpenter <brandon.carpenter@cypherpath.com>
    Fixes: a6e18ff1 ("vlan: Fix untag operations of stacked vlans with REORDER_HEADER off")
    Signed-off-by: default avatarToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    4bbb3e0e
skbuff.c 136 KB