tcp: fix race condition when creating child sockets from syncookies

Message ID	20201023111352.GA289522@rdias-suse-pc.lan
State	New
Headers	show Return-Path: <SRS0=QbkO=D6=vger.kernel.org=netdev-owner@kernel.org> Date: Fri, 23 Oct 2020 12:13:52 +0100 From: Ricardo Dias <rdias@memsql.com> To: davem@davemloft.net, kuba@kernel.org, kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org, edumazet@google.com Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] tcp: fix race condition when creating child sockets from syncookies Message-ID: <20201023111352.GA289522@rdias-suse-pc.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Precedence: bulk
Series	tcp: fix race condition when creating child sockets from syncookies \| expand tcp: fix race condition when creating child sockets from syncookies

diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h index 92560974ea67..e1d7fd20468a 100644 --- a/include/net/inet_hashtables.h +++ b/include/net/inet_hashtables.h @@ -248,6 +248,7 @@ void inet_hashinfo2_init(struct inet_hashinfo *h, const char *name, int inet_hashinfo2_init_mod(struct inet_hashinfo *h); bool inet_ehash_insert(struct sock *sk, struct sock *osk); +struct sock *inet_ehash_insert_chk_dup(struct sock *sk); bool inet_ehash_nolisten(struct sock *sk, struct sock *osk); int __inet_hash(struct sock *sk, struct sock *osk); int inet_hash(struct sock *sk); diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c index 239e54474b65..5dbe3aa291e6 100644 --- a/net/ipv4/inet_hashtables.c +++ b/net/ipv4/inet_hashtables.c @@ -539,6 +539,65 @@ bool inet_ehash_insert(struct sock *sk, struct sock *osk) return ret; } +/* Inserts a socket into ehash if no existing socket exists for the same + * quadruple (saddr, sport, daddr, dport). + * If there is an existing socket, returns that socket, otherwise returns NULL. + */ +struct sock *inet_ehash_insert_chk_dup(struct sock *sk) +{ + struct inet_hashinfo *hashinfo = sk->sk_prot->h.hashinfo; + struct hlist_nulls_head *list; + struct inet_ehash_bucket *head; + const struct hlist_nulls_node *node; + struct sock *esk; + spinlock_t *lock; /* protects hashinfo socket entry */ + struct net *net = sock_net(sk); + const int dif, sdif = sk->sk_bound_dev_if; + + INET_ADDR_COOKIE(acookie, sk->sk_daddr, sk->sk_rcv_saddr); + const __portpair ports = INET_COMBINED_PORTS(sk->sk_dport, sk->sk_num); + + WARN_ON_ONCE(!sk_unhashed(sk)); + + sk->sk_hash = sk_ehashfn(sk); + head = inet_ehash_bucket(hashinfo, sk->sk_hash); + list = &head->chain; + lock = inet_ehash_lockp(hashinfo, sk->sk_hash); + + spin_lock(lock); +begin: + sk_nulls_for_each_rcu(esk, node, list) { + if (esk->sk_hash != sk->sk_hash) + continue; + if (likely(INET_MATCH(esk, net, acookie, + sk->sk_daddr, sk->sk_rcv_saddr, ports, + dif, sdif))) { + if (unlikely(!refcount_inc_not_zero(&esk->sk_refcnt))) + goto out; + if (unlikely(!INET_MATCH(esk, net, acookie, + sk->sk_daddr, + sk->sk_rcv_saddr, ports, + dif, sdif))) { + sock_gen_put(esk); + goto begin; + } + goto found; + } + } +out: + esk = NULL; + __sk_nulls_add_node_rcu(sk, list); +found: + spin_unlock(lock); + if (esk) { + percpu_counter_inc(sk->sk_prot->orphan_count); + inet_sk_set_state(sk, TCP_CLOSE); + sock_set_flag(sk, SOCK_DEAD); + inet_csk_destroy_sock(sk); + } + return esk; +} + bool inet_ehash_nolisten(struct sock *sk, struct sock *osk) { bool ok = inet_ehash_insert(sk, osk); diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c index e03756631541..c4bb895085f0 100644 --- a/net/ipv4/syncookies.c +++ b/net/ipv4/syncookies.c @@ -208,7 +208,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb, child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst, NULL, &own_req); - if (child) { + if (child && own_req) { refcount_set(&req->rsk_refcnt, 1); tcp_sk(child)->tsoffset = tsoff; sock_rps_save_rxhash(child, skb); @@ -223,6 +223,9 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb, bh_unlock_sock(child); sock_put(child); + } else if (child && !own_req) { + __reqsk_free(req); + return child; } __reqsk_free(req); diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 592c73962723..c705d335bd80 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1501,6 +1501,8 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb, int l3index; #endif struct ip_options_rcu *inet_opt; + struct sock *esk = NULL; + bool syncookie = false; if (sk_acceptq_is_full(sk)) goto exit_overflow; @@ -1535,6 +1537,7 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb, goto put_and_exit; } else { /* syncookie case : see end of cookie_v4_check() */ + syncookie = true; } sk_setup_caps(newsk, dst); @@ -1565,7 +1568,18 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb, if (__inet_inherit_port(sk, newsk) < 0) goto put_and_exit; - *own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash)); + if (!syncookie) { + *own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash)); + } else { + esk = inet_ehash_insert_chk_dup(newsk); + /* We're going to notify tcp_get_cookie_sock through own_req + * that an existing socket is going to be returned instead, + * since own_req is not used by the syncookies code. + */ + *own_req = !esk; + if (esk) + newsk = esk; + } if (likely(*own_req)) { tcp_move_syn(newtp, req); ireq->ireq_opt = NULL;

tcp: fix race condition when creating child sockets from syncookies

Commit Message

Comments

Patch