dccp/tcp: Reset saddr on failure after inet6?_hash_connect().

When connect() is called on a socket bound to the wildcard address,
we change the socket's saddr to a local address.  If the socket
fails to connect() to the destination, we have to reset the saddr.

However, when an error occurs after inet_hash6?_connect() in
(dccp|tcp)_v[46]_conect(), we forget to reset saddr and leave
the socket bound to the address.

From the user's point of view, whether saddr is reset or not varies
with errno.  Let's fix this inconsistent behaviour.

Note that after this patch, the repro [0] will trigger the WARN_ON()
in inet_csk_get_port() again, but this patch is not buggy and rather
fixes a bug papering over the bhash2's bug for which we need another
fix.

For the record, the repro causes -EADDRNOTAVAIL in inet_hash6_connect()
by this sequence:

  s1 = socket()
  s1.setsockopt(SOL_SOCKET, SO_REUSEADDR, 1)
  s1.bind(('127.0.0.1', 10000))
  s1.sendto(b'hello', MSG_FASTOPEN, (('127.0.0.1', 10000)))
  # or s1.connect(('127.0.0.1', 10000))

  s2 = socket()
  s2.setsockopt(SOL_SOCKET, SO_REUSEADDR, 1)
  s2.bind(('0.0.0.0', 10000))
  s2.connect(('127.0.0.1', 10000))  # -EADDRNOTAVAIL

  s2.listen(32)  # WARN_ON(inet_csk(sk)->icsk_bind2_hash != tb2);

[0]: https://syzkaller.appspot.com/bug?extid=015d756bbd1f8b5c8f09

Fixes: 3df80d9320bc ("[DCCP]: Introduce DCCPv6")
Fixes: 7c657876b63c ("[DCCP]: Initial implementation")
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Acked-by: Joanne Koong <joannelkoong@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 713b7b8dad7e58844e67da727b831d0f9f28dc38..40640c26680e05be233d3f165f97dca44f325229 100644 (file)
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -157,6 +157,8 @@ failure:
         * This unhashes the socket and releases the local port, if necessary.
         */
        dccp_set_state(sk, DCCP_CLOSED);
+       if (!(sk->sk_userlocks & SOCK_BINDADDR_LOCK))
+               inet_reset_saddr(sk);
        ip_rt_put(rt);
        sk->sk_route_caps = 0;
        inet->inet_dport = 0;

diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index e57b43006074627776c8119a9b349d99ea27b1d4..626166cb6d7e55e6e98bf4e61ec5e862ad0d5c44 100644 (file)
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -985,6 +985,8 @@ static int dccp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
 
 late_failure:
        dccp_set_state(sk, DCCP_CLOSED);
+       if (!(sk->sk_userlocks & SOCK_BINDADDR_LOCK))
+               inet_reset_saddr(sk);
        __sk_dst_reset(sk);
 failure:
        inet->inet_dport = 0;

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 87d440f47a703bb5aa4736551cac3597935290e5..6a3a732b584d26ad805b08d7f0c0e429eebb4be4 100644 (file)
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -343,6 +343,8 @@ failure:
         * if necessary.
         */
        tcp_set_state(sk, TCP_CLOSE);
+       if (!(sk->sk_userlocks & SOCK_BINDADDR_LOCK))
+               inet_reset_saddr(sk);
        ip_rt_put(rt);
        sk->sk_route_caps = 0;
        inet->inet_dport = 0;

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 2a3f9296df1e505b40e925c31b0d2aa2a2327cfd..81b396e5cf7983526ae6bed706e2bc23e487770b 100644 (file)
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -359,6 +359,8 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
 
 late_failure:
        tcp_set_state(sk, TCP_CLOSE);
+       if (!(sk->sk_userlocks & SOCK_BINDADDR_LOCK))
+               inet_reset_saddr(sk);
 failure:
        inet->inet_dport = 0;
        sk->sk_route_caps = 0;