netdev - Re: Sending 802.1Q packets using AF_PACKET socket on filtered bridge forwards with wrong MAC addresses

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 6 Dec 2017 13:44:03 +0900
From:   Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
To:     Brandon Carpenter <brandon.carpenter@...herpath.com>
Cc:     Stephen Hemminger <stephen@...workplumber.org>,
        "David S. Miller" <davem@...emloft.net>,
        bridge@...ts.linux-foundation.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Vlad Yasevich <vyasevic@...hat.com>
Subject: Re: Sending 802.1Q packets using AF_PACKET socket on filtered bridge
 forwards with wrong MAC addresses

Hi,
(CC: Vlad)

On 2017/11/30 7:01, Brandon Carpenter wrote:
> I narrowed the search to a memmove() called from
> skb_reorder_vlan_header() in net/core/skbuff.c.
> 
>>     memmove(skb->data - ETH_HLEN, skb->data - skb->mac_len - VLAN_HLEN,
>>        2 * ETH_ALEN);
> 
> Calling skb_reset_mac_len() after skb_reset_mac_header() before
> calling br_allowed_ingress() in net/bridge/br_device.c fixes the
> problem.
> 
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index af5b8c87f590..e10131e2f68f 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -58,6 +58,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct
> net_device *dev)
>         BR_INPUT_SKB_CB(skb)->brdev = dev;
> 
>         skb_reset_mac_header(skb);
> +       skb_reset_mac_len(skb);
>         eth = eth_hdr(skb);
>         skb_pull(skb, ETH_HLEN);

Thanks for debugging this problem.
It seems this has been broken since a6e18ff11170 ("vlan: Fix untag
operations of stacked vlans with REORDER_HEADER off").

Unfortunately this does not always work correctly, since in tx path
drivers assume network header to be set to L3 protocol header offset.
Packet socket (packet_snd()) determines network header by
dev_hard_header which is ETH_HLEN in bridge devices, so this works for
packet socket, but with vlan devices on top of bridge device with
tx-vlan hwaccel disabled we get ETH_HLEN + VLAN_HLEN or longer by mac_len.

Since mac_len can be arbitrarily long if we stack vlan devices on bridge
devices, and since we want to untag the outermost tag, using mac_len to
untag in tx path is probably no longer correct.

I'll think deeper about how to fix it.

> I'll put together an official patch  and submit it. Should I use
> another email account? Are my emails being ignored because of that
> stupid disclaimer my employer attaches to my messages (outside my
> control)?
> 
> Brandon
> 

-- 
Toshiaki Makita