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

Message-ID: <CA+FuTSeq5fkUruH8OPeV1kBAV9p-unD3i8WfcHZK+-6wf8p03w@mail.gmail.com>
Date:   Mon, 17 Feb 2020 18:12:54 -0800
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        Pavel Roskin <plroskin@...il.com>,
        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 Mon, Feb 17, 2020 at 5:24 PM Eric Dumazet <eric.dumazet@...il.com> wrote:
>
>
>
> 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.

Agreed. But it seems to go back to at least 2003.

>
> > -       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;
> }

Ah yes, thanks.