netdev - Re: [PATCH net,stable] tun: fix vlan packet truncation

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <bc626bc5-5d48-ef8e-29d8-56f401a054eb@redhat.com>
Date:   Tue, 17 Apr 2018 10:13:45 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Bjørn Mork <bjorn@...k.no>, netdev@...r.kernel.org
Subject: Re: [PATCH net,stable] tun: fix vlan packet truncation



On 2018年04月17日 06:00, Bjørn Mork wrote:
> Bogus trimming in tun_net_xmit() causes truncated vlan packets.
>
> skb->len is correct whether or not skb_vlan_tag_present() is true. There
> is no more reason to adjust the skb length on xmit in this driver than
> any other driver. tun_put_user() adds 4 bytes to the total for tagged
> packets because it transmits the tag inline to userspace.  This is
> similar to a nic transmitting the tag inline on the wire.
>
> Reproducing the bug by sending any tagged packet through back-to-back
> connected tap interfaces:
>
>   socat TUN,tun-type=tap,iff-up,tun-name=in TUN,tun-type=tap,iff-up,tun-name=out &
>   ip link add link in name in.20 type vlan id 20
>   ip addr add 10.9.9.9/24 dev in.20
>   ip link set in.20 up
>   tshark -nxxi in -f arp -c1 2>/dev/null &
>   tshark -nxxi out -f arp -c1 2>/dev/null &
>   ping -c 1 10.9.9.5 >/dev/null 2>&1
>
> The output from the 'in' and 'out' interfaces are different when the
> bug is present:
>
>   Capturing on 'in'
>   0000  ff ff ff ff ff ff 76 cf 76 37 d5 0a 81 00 00 14   ......v.v7......
>   0010  08 06 00 01 08 00 06 04 00 01 76 cf 76 37 d5 0a   ..........v.v7..
>   0020  0a 09 09 09 00 00 00 00 00 00 0a 09 09 05         ..............
>
>   Capturing on 'out'
>   0000  ff ff ff ff ff ff 76 cf 76 37 d5 0a 81 00 00 14   ......v.v7......
>   0010  08 06 00 01 08 00 06 04 00 01 76 cf 76 37 d5 0a   ..........v.v7..
>   0020  0a 09 09 09 00 00 00 00 00 00                     ..........
>
> Fixes: aff3d70a07ff ("tun: allow to attach ebpf socket filter")
> Cc: Jason Wang <jasowang@...hat.com>
> Signed-off-by: Bjørn Mork <bjorn@...k.no>
> ---
>   drivers/net/tun.c | 7 -------
>   1 file changed, 7 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 28583aa0c17d..01cf8e3d8edc 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1103,13 +1103,6 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>   
>   	len = run_ebpf_filter(tun, skb, len);
>   
> -	/* Trim extra bytes since we may insert vlan proto & TCI
> -	 * in tun_put_user().
> -	 */
> -	len -= skb_vlan_tag_present(skb) ? sizeof(struct veth) : 0;
> -	if (len <= 0 || pskb_trim(skb, len))
> -		goto drop;
> -
>   	if (unlikely(skb_orphan_frags_rx(skb, GFP_ATOMIC)))
>   		goto drop;
>   

Good catch, but I still think we should do the truncation in 
run_ebpf_filter to match the behavior socket ebpf filter.

Thanks