| From foo@baz Sun May 27 17:33:38 CEST 2018 |
| From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> |
| Date: Tue, 13 Mar 2018 14:51:27 +0900 |
| Subject: net: Fix vlan untag for bridge and vlan_dev with reorder_hdr off |
| |
| From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> |
| |
| [ Upstream commit 4bbb3e0e8239f9079bf1fe20b3c0cb598714ae61 ] |
| |
| 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: Brandon Carpenter <brandon.carpenter@cypherpath.com> |
| Fixes: a6e18ff11170 ("vlan: Fix untag operations of stacked vlans with REORDER_HEADER off") |
| Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| include/uapi/linux/if_ether.h | 1 + |
| net/core/skbuff.c | 7 +++++-- |
| 2 files changed, 6 insertions(+), 2 deletions(-) |
| |
| --- a/include/uapi/linux/if_ether.h |
| +++ b/include/uapi/linux/if_ether.h |
| @@ -29,6 +29,7 @@ |
| */ |
| |
| #define ETH_ALEN 6 /* Octets in one ethernet addr */ |
| +#define ETH_TLEN 2 /* Octets in ethernet type field */ |
| #define ETH_HLEN 14 /* Total octets in header. */ |
| #define ETH_ZLEN 60 /* Min. octets in frame sans FCS */ |
| #define ETH_DATA_LEN 1500 /* Max. octets in payload */ |
| --- a/net/core/skbuff.c |
| +++ b/net/core/skbuff.c |
| @@ -4475,13 +4475,16 @@ EXPORT_SYMBOL_GPL(skb_gso_validate_mtu); |
| |
| static struct sk_buff *skb_reorder_vlan_header(struct sk_buff *skb) |
| { |
| + int mac_len; |
| + |
| if (skb_cow(skb, skb_headroom(skb)) < 0) { |
| kfree_skb(skb); |
| return NULL; |
| } |
| |
| - memmove(skb->data - ETH_HLEN, skb->data - skb->mac_len - VLAN_HLEN, |
| - 2 * ETH_ALEN); |
| + mac_len = skb->data - skb_mac_header(skb); |
| + memmove(skb_mac_header(skb) + VLAN_HLEN, skb_mac_header(skb), |
| + mac_len - VLAN_HLEN - ETH_TLEN); |
| skb->mac_header += VLAN_HLEN; |
| return skb; |
| } |