dccp/tcp: Reset saddr on failure after inet6?_hash_connect().
authorKuniyuki Iwashima <kuniyu@amazon.com>
Sat, 19 Nov 2022 01:49:11 +0000 (17:49 -0800)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 2 Dec 2022 16:41:07 +0000 (17:41 +0100)
[ Upstream commit 77934dc6db0d2b111a8f2759e9ad2fb67f5cffa5 ]

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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
net/dccp/ipv4.c
net/dccp/ipv6.c
net/ipv4/tcp_ipv4.c
net/ipv6/tcp_ipv6.c

index 0ea29270d7e53730d14ec43654be8f956f891552..5bcfa1e9a941bd3a16d2e5fdceca38fcb8447b44 100644 (file)
@@ -137,6 +137,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;
index fa663518fa0e465458b7486ad0cd0672425f08b0..071620622e1e1f03aa55ea7765cfa711d2a091f5 100644 (file)
@@ -967,6 +967,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;
index 42d4af6324950ac6fcda893b5fb98666ebc6a3bb..0e1fbad17dbe3ca5b53c8d2acba5eb76c17f69e2 100644 (file)
@@ -324,6 +324,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;
index 51f4d330e820bb1e887fa531a19e979a33951774..93b3e7c247cec0a7f218e8e8604eb3014a430c85 100644 (file)
@@ -339,6 +339,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;