netdev - Re: [BISECTED] UDP socket bound to addr_any receives no data after disconnect

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]

Date:   Mon, 17 Feb 2020 17:24:08 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        Pavel Roskin <plroskin@...il.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        Peter Oskolkov <posk@...gle.com>,
        Eric Dumazet <edumazet@...gle.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [BISECTED] UDP socket bound to addr_any receives no data after
 disconnect



On 2/17/20 3:20 PM, Willem de Bruijn wrote:
> On Sun, Feb 16, 2020 at 10:53 AM Pavel Roskin <plroskin@...il.com> wrote:
>>
>> Hello,
>>
>> I was debugging a program that uses UDP to serve one client at a time.
>> It stopped working on newer Linux versions. I was able to bisect the
>> issue to commit 4cdeeee9252af1ba50482f91d615f326365306bd, "net: udp:
>> prefer listeners bound to an address". The commit is present in Linux
>> 5.0 but not in 4.20. Linux 5.5.4 is still affected.
>>
>> From reading the commit description, it doesn't appear that the effect
>> is intended. However, I found that the issue goes away if I bind the
>> socket to the loopback address.
>>
>> I wrote a demo program that shows the problem:
>>
>> server binds to 0.0.0.0:1337
>> server connects to 127.0.0.1:80
>> server disconnects
>> client connects to 127.0.0.1:1337
>> client sends "hello"
>> server gets nothing
>>
>> Load a 4.x kernel, and the server would get "hello". Likewise, change
>> "0.0.0.0" to "127.0.0.1" and the problem goes away.
>>
>> IPv6 has the same issue. I'm attaching programs that demonstrate the
>> issue with IPv4 and IPv6. They print "hello" on success and hang
>> otherwise.
> 
> Thanks for the report with reproducers. That's very helpful.
> 
> Before the patch, __udp4_lib_lookup looks into the hslot table hashed
> only by destination port.
> 
> After the patch, it goes to hslot2, hashed by dport and daddr. Before
> the connect and disconnect calls, the server socket is hashed on
> INADDR_ANY.
> 
> The connect call changes inet_rcv_saddr and calls sk_prot->rehash to
> move the socket to the hslot hashed on its saddr matching the new
> destination.
> 
> The disconnect call reverts inet_rcv_saddr to INADDR_ANY, but lacks a
> rehash. The following makes your ipv4 test pass:
> 
> @@ -1828,8 +1828,11 @@ int __udp_disconnect(struct sock *sk, int flags)
>         inet->inet_dport = 0;
>         sock_rps_reset_rxhash(sk);
>         sk->sk_bound_dev_if = 0;

BTW, any idea why sk_bound_dev_if is cleared ?

This looks orthogonal to a disconnect.

> -       if (!(sk->sk_userlocks & SOCK_BINDADDR_LOCK))
> +       if (!(sk->sk_userlocks & SOCK_BINDADDR_LOCK)) {
>                 inet_reset_saddr(sk);
> +               if (sk->sk_prot->rehash)
> +                       sk->sk_prot->rehash(sk);
> +       }

Note that we might call sk->sk_prot->unhash(sk) right after this point,
so maybe avoid a rehash unless really needed.

if (!(sk->sk_userlocks & SOCK_BINDPORT_LOCK)) {
        sk->sk_prot->unhash(sk);
        inet->inet_sport = 0;
}