TCP established ehash lookups are lockless. The socket caches involved
use SLAB_TYPESAFE_BY_RCU, so memory can be reused before an RCU reader
that retained its old address has finished.
After a tuple match, __inet_lookup_established() and
__inet6_lookup_established() acquire a reference and repeat the tuple
check. They do not check whether the new object is hashed. If the
address is reused for a socket with the same tuple, the lookup can
acquire a reference to the new object and return it while its nulls node
is unhashed.
A PoC that drives reuse with a SYN cookie request socket reproduced the
crash. The new request socket is unhashed and has a NULL rsk_listener.
A lookup that had loaded the address of the old request socket acquired
the reference published by tcp_get_cookie_sock() and returned the new
object. tcp_v4_rcv() then read sk_state through the NULL listener.
The resulting crash was:
BUG: kernel NULL pointer dereference, address: 0000000000000012
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
...
Oops: Oops: 0000 [#1] SMP NOPTI
CPU: 4 UID: 0 PID: 0 Comm: swapper/4 Not tainted 7.3.0-00591-g0e135910deec #1 PREEMPT(lazy)
...
RIP: 0010:tcp_v4_rcv+0x44e/0x1280
...
R10: 0000000000000001 R11: 00000000024ca8c0 R12: 0000000000000000
...
CR2: 0000000000000012 CR3: 0000000103d50004 CR4: 0000000000770ef0
...
Call Trace:
? raw_local_deliver+0xae/0x240
ip_protocol_deliver_rcu+0x2d/0x180
ip_local_deliver_finish+0x84/0x100
ip_sublist_rcv_finish+0x5f/0x70
ip_sublist_rcv+0x12a/0x1d0
ip_list_rcv+0xfd/0x130
__netif_receive_skb_list_core+0x223/0x250
netif_receive_skb_list_internal+0x1d2/0x2c0
? try_fill_recv+0x39/0x7b0
napi_complete_done+0x74/0x170
virtnet_poll+0x445/0x600
__napi_poll+0x27/0x1e0
net_rx_action+0x2fe/0x3b0
handle_softirqs+0xc2/0x2c0
__irq_exit_rcu+0x74/0xd0
common_interrupt+0x7f/0x90
...
Kernel panic - not syncing: Fatal exception in interrupt
After taking a reference, check whether the nulls node is unhashed in
both IPv4 and IPv6 established lookups. If it is unhashed or the tuple
no longer matches, drop the reference and restart the lookup.
Lookups racing with the TIME_WAIT hashdance, or with a request being
replaced by its child, now restart and find the new socket instead of
returning the old one.
Request sockets that were never inserted into ehash can have a nonzero
refcount, and a stale lookup can hold a transient reference. Therefore,
when SYN cookie, Fast Open, and request migration paths fail after
setting a nonzero refcount, release the references held by those paths
with reqsk_put() instead of freeing the request directly.
Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim
---
net/ipv4/inet_connection_sock.c | 10 +++++++---
net/ipv4/inet_hashtables.c | 5 +++--
net/ipv4/syncookies.c | 2 ++
net/ipv4/tcp_input.c | 4 +++-
net/ipv6/inet6_hashtables.c | 4 +++-
5 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 6a30f1138454..cff02f4c5ac8 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -1136,10 +1136,14 @@ static void reqsk_timer_handler(struct timer_list *t)
*/
if (nreq) {
__NET_INC_STATS(net, LINUX_MIB_TCPMIGRATEREQFAILURE);
+ /* The timer was not armed, drop its reference here. */
+ reqsk_put(nreq);
no_ownership:
reqsk_migrate_reset(nreq);
reqsk_queue_removed(queue, nreq);
- __reqsk_free(nreq);
+ /* Drop the ehash and temporary references. */
+ reqsk_put(nreq);
+ reqsk_put(nreq);
}
drop:
@@ -1447,7 +1451,7 @@ struct sock *inet_csk_complete_hashdance(struct sock *sk, struct sock *child,
__NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMIGRATEREQFAILURE);
reqsk_migrate_reset(nreq);
- __reqsk_free(nreq);
+ reqsk_put(nreq);
} else if (inet_csk_reqsk_queue_add(sk, req, child)) {
return child;
}
@@ -1502,7 +1506,7 @@ void inet_csk_listen_stop(struct sock *sk)
__NET_INC_STATS(sock_net(nsk),
LINUX_MIB_TCPMIGRATEREQFAILURE);
reqsk_migrate_reset(nreq);
- __reqsk_free(nreq);
+ reqsk_put(nreq);
}
rcu_read_unlock();
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index ba0faa9ae2bb..58d54965036a 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -552,8 +552,9 @@ struct sock *__inet_lookup_established(const struct net *net,
if (likely(inet_match(net, sk, acookie, ports, dif, sdif))) {
if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
goto out;
- if (unlikely(!inet_match(net, sk, acookie,
- ports, dif, sdif))) {
+ if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
+ !inet_match(net, sk, acookie, ports, dif,
+ sdif))) {
sock_gen_put(sk);
goto begin;
}
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 73e129768184..644030561537 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -218,6 +218,8 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
bh_unlock_sock(child);
sock_put(child);
+ reqsk_put(req);
+ return NULL;
}
__reqsk_free(req);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 99baf14afdfd..3583867487d7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -7777,7 +7777,9 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
if (!inet_csk_reqsk_queue_add(sk, req, fastopen_sk)) {
bh_unlock_sock(fastopen_sk);
sock_put(fastopen_sk);
- goto drop_and_free;
+ /* tcp_disconnect() already dropped one reqsk ref. */
+ reqsk_put(req);
+ goto drop;
}
READ_ONCE(sk->sk_data_ready)(sk);
bh_unlock_sock(fastopen_sk);
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index b111b51d69fc..aa5fa1dd062f 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -108,7 +108,9 @@ struct sock *__inet6_lookup_established(const struct net *net,
if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
goto out;
- if (unlikely(!inet6_match(net, sk, saddr, daddr, ports, dif, sdif))) {
+ if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
+ !inet6_match(net, sk, saddr, daddr, ports, dif,
+ sdif))) {
sock_gen_put(sk);
goto begin;
}
--
2.43.0