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 15:20:59 -0800
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     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 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;
-       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);
+       }