netdev - Re: [PATCH net] ip: hash fragments consistently

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: <20180723.114018.2252598987977006948.davem@davemloft.net>
Date:   Mon, 23 Jul 2018 11:40:18 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     pabeni@...hat.com
Cc:     netdev@...r.kernel.org, tom@...bertland.com
Subject: Re: [PATCH net] ip: hash fragments consistently

From: Paolo Abeni <pabeni@...hat.com>
Date: Mon, 23 Jul 2018 16:50:48 +0200

> The skb hash for locally generated ip[v6] fragments belonging
> to the same datagram can vary in several circumstances:
> * for connected UDP[v6] sockets, the first fragment get its hash
>   via set_owner_w()/skb_set_hash_from_sk()
> * for unconnected IPv6 UDPv6 sockets, the first fragment can get
>   its hash via ip6_make_flowlabel()/skb_get_hash_flowi6(), if
>   auto_flowlabel is enabled
> 
> For the following frags the hash is usually computed via
> skb_get_hash().
> The above can cause OoO for unconnected IPv6 UDPv6 socket: in that
> scenario the egress tx queue can be selected on a per packet basis
> via the skb hash.
> It may also fool flow-oriented schedulers to place fragments belonging
> to the same datagram in different flows.
> 
> Fix the issue by copying the skb hash from the head frag into
> the others at fragmentation time.
> 
> Before this commit:
> perf probe -a "dev_queue_xmit skb skb->hash skb->l4_hash:b1@0/8 skb->sw_hash:b1@1/8"
> netperf -H $IPV4 -t UDP_STREAM -l 5 -- -m 2000 -n &
> perf record -e probe:dev_queue_xmit -e probe:skb_set_owner_w -a sleep 0.1
> perf script
> probe:dev_queue_xmit: (ffffffff8c6b1b20) hash=3713014309 l4_hash=1 sw_hash=0
> probe:dev_queue_xmit: (ffffffff8c6b1b20) hash=0 l4_hash=0 sw_hash=0
> 
> After this commit:
> probe:dev_queue_xmit: (ffffffff8c6b1b20) hash=2171763177 l4_hash=1 sw_hash=0
> probe:dev_queue_xmit: (ffffffff8c6b1b20) hash=2171763177 l4_hash=1 sw_hash=0
> 
> Fixes: b73c3d0e4f0e ("net: Save TX flow hash in sock and set in skbuf on xmit")
> Fixes: 67800f9b1f4e ("ipv6: Call skb_get_hash_flowi6 to get skb->hash in ip6_make_flowlabel")
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>

Good catch!

Applied and queued up for -stable, thanks!