tcp/dccp: fix possible race __inet_lookup_established()

BugLink: https://bugs.launchpad.net/bugs/1860602
commit 8dbd76e79a16b45b2ccb01d2f2e08dbf64e71e40 upstream.

Michal Kubecek and Firo Yang did a very nice analysis of crashes
happening in __inet_lookup_established().

Since a TCP socket can go from TCP_ESTABLISH to TCP_LISTEN
(via a close()/socket()/listen() cycle) without a RCU grace period,
I should not have changed listeners linkage in their hash table.

They must use the nulls protocol (Documentation/RCU/rculist_nulls.txt),
so that a lookup can detect a socket in a hash list was moved in
another one.

Since we added code in commit d296ba60d8e2 ("soreuseport: Resolve
merge conflict for v4/v6 ordering fix"), we have to add
hlist_nulls_add_tail_rcu() helper.

Fixes: 3b24d854cb35 ("tcp/dccp: do not touch listener sk_refcnt under synflood")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Michal Kubecek <mkubecek@suse.cz>
Reported-by: Firo Yang <firo.yang@suse.com>
Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
Link: https://lore.kernel.org/netdev/20191120083919.GH27852@unicorn.suse.cz/
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
[stable-4.14: we also need to update code in __inet_lookup_listener() and
 inet6_lookup_listener() which has been removed in 5.0-rc1.]
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>

diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
index e4b257ff881bfe439a945d7487f5700f17a26740..a10da545b3f670c91caced56b85200f788403dd5 100644 (file)
--- a/include/linux/rculist_nulls.h
+++ b/include/linux/rculist_nulls.h
@@ -100,6 +100,43 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
                first->pprev = &n->next;
 }
 
+/**
+ * hlist_nulls_add_tail_rcu
+ * @n: the element to add to the hash list.
+ * @h: the list to add to.
+ *
+ * Description:
+ * Adds the specified element to the specified hlist_nulls,
+ * while permitting racing traversals.
+ *
+ * The caller must take whatever precautions are necessary
+ * (such as holding appropriate locks) to avoid racing
+ * with another list-mutation primitive, such as hlist_nulls_add_head_rcu()
+ * or hlist_nulls_del_rcu(), running on this same list.
+ * However, it is perfectly legal to run concurrently with
+ * the _rcu list-traversal primitives, such as
+ * hlist_nulls_for_each_entry_rcu(), used to prevent memory-consistency
+ * problems on Alpha CPUs.  Regardless of the type of CPU, the
+ * list-traversal primitive must be guarded by rcu_read_lock().
+ */
+static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
+                                           struct hlist_nulls_head *h)
+{
+       struct hlist_nulls_node *i, *last = NULL;
+
+       /* Note: write side code, so rcu accessors are not needed. */
+       for (i = h->first; !is_a_nulls(i); i = i->next)
+               last = i;
+
+       if (last) {
+               n->next = last->next;
+               n->pprev = &last->next;
+               rcu_assign_pointer(hlist_next_rcu(last), n);
+       } else {
+               hlist_nulls_add_head_rcu(n, h);
+       }
+}
+
 /**
  * hlist_nulls_for_each_entry_rcu - iterate over rcu list of given type
  * @tpos:      the type * to use as a loop cursor.

diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index 2dbbbff5e1e3aa58434bbafa691d44005b3e2450..573ab110c9ec8920a5c94e596b905a9e9aadaa99 100644 (file)
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -106,12 +106,18 @@ struct inet_bind_hashbucket {
        struct hlist_head       chain;
 };
 
-/*
- * Sockets can be hashed in established or listening table
+/* Sockets can be hashed in established or listening table.
+ * We must use different 'nulls' end-of-chain value for all hash buckets :
+ * A socket might transition from ESTABLISH to LISTEN state without
+ * RCU grace period. A lookup in ehash table needs to handle this case.
  */
+#define LISTENING_NULLS_BASE (1U << 29)
 struct inet_listen_hashbucket {
        spinlock_t              lock;
-       struct hlist_head       head;
+       union {
+               struct hlist_head       head;
+               struct hlist_nulls_head nulls_head;
+       };
 };
 
 /* This is for listening sockets, thus all sockets which possess wildcards. */

diff --git a/include/net/sock.h b/include/net/sock.h
index e01c579d57539998d2a98afa64a407c26b104f91..70acf56a0fbc113929c6912ecb750ed08b0a2f3d 100644 (file)
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -698,6 +698,11 @@ static inline void __sk_nulls_add_node_rcu(struct sock *sk, struct hlist_nulls_h
        hlist_nulls_add_head_rcu(&sk->sk_nulls_node, list);
 }
 
+static inline void __sk_nulls_add_node_tail_rcu(struct sock *sk, struct hlist_nulls_head *list)
+{
+       hlist_nulls_add_tail_rcu(&sk->sk_nulls_node, list);
+}
+
 static inline void sk_nulls_add_node_rcu(struct sock *sk, struct hlist_nulls_head *list)
 {
        sock_hold(sk);

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 33edccfebc304b17c48f1c50b45bf5d2540a1c6b..eb158badebc43ece6b804d1aaa7a48ec2dad1c80 100644 (file)
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -911,11 +911,12 @@ void inet_diag_dump_icsk(struct inet_hashinfo *hashinfo, struct sk_buff *skb,
 
                for (i = s_i; i < INET_LHTABLE_SIZE; i++) {
                        struct inet_listen_hashbucket *ilb;
+                       struct hlist_nulls_node *node;
 
                        num = 0;
                        ilb = &hashinfo->listening_hash[i];
                        spin_lock(&ilb->lock);
-                       sk_for_each(sk, &ilb->head) {
+                       sk_nulls_for_each(sk, node, &ilb->nulls_head) {
                                struct inet_sock *inet = inet_sk(sk);
 
                                if (!net_eq(sock_net(sk), net))

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 1f26627c7fadbae49b1b3c020997253ac5a1829b..0af13f5bdc9a3ac48b42875a593c51bc7cd3d1fa 100644 (file)
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -219,9 +219,10 @@ struct sock *__inet_lookup_listener(struct net *net,
        int score, hiscore = 0, matches = 0, reuseport = 0;
        bool exact_dif = inet_exact_dif_match(net, skb);
        struct sock *sk, *result = NULL;
+       struct hlist_nulls_node *node;
        u32 phash = 0;
 
-       sk_for_each_rcu(sk, &ilb->head) {
+       sk_nulls_for_each_rcu(sk, node, &ilb->nulls_head) {
                score = compute_score(sk, net, hnum, daddr,
                                      dif, sdif, exact_dif);
                if (score > hiscore) {
@@ -442,10 +443,11 @@ static int inet_reuseport_add_sock(struct sock *sk,
                                   struct inet_listen_hashbucket *ilb)
 {
        struct inet_bind_bucket *tb = inet_csk(sk)->icsk_bind_hash;
+       const struct hlist_nulls_node *node;
        struct sock *sk2;
        kuid_t uid = sock_i_uid(sk);
 
-       sk_for_each_rcu(sk2, &ilb->head) {
+       sk_nulls_for_each_rcu(sk2, node, &ilb->nulls_head) {
                if (sk2 != sk &&
                    sk2->sk_family == sk->sk_family &&
                    ipv6_only_sock(sk2) == ipv6_only_sock(sk) &&
@@ -480,9 +482,9 @@ int __inet_hash(struct sock *sk, struct sock *osk)
        }
        if (IS_ENABLED(CONFIG_IPV6) && sk->sk_reuseport &&
                sk->sk_family == AF_INET6)
-               hlist_add_tail_rcu(&sk->sk_node, &ilb->head);
+               __sk_nulls_add_node_tail_rcu(sk, &ilb->nulls_head);
        else
-               hlist_add_head_rcu(&sk->sk_node, &ilb->head);
+               __sk_nulls_add_node_rcu(sk, &ilb->nulls_head);
        sock_set_flag(sk, SOCK_RCU_FREE);
        sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
 unlock:
@@ -525,10 +527,7 @@ void inet_unhash(struct sock *sk)
        spin_lock_bh(lock);
        if (rcu_access_pointer(sk->sk_reuseport_cb))
                reuseport_detach_sock(sk);
-       if (listener)
-               done = __sk_del_node_init(sk);
-       else
-               done = __sk_nulls_del_node_init_rcu(sk);
+       done = __sk_nulls_del_node_init_rcu(sk);
        if (done)
                sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
        spin_unlock_bh(lock);
@@ -664,7 +663,8 @@ void inet_hashinfo_init(struct inet_hashinfo *h)
 
        for (i = 0; i < INET_LHTABLE_SIZE; i++) {
                spin_lock_init(&h->listening_hash[i].lock);
-               INIT_HLIST_HEAD(&h->listening_hash[i].head);
+               INIT_HLIST_NULLS_HEAD(&h->listening_hash[i].nulls_head,
+                                     i + LISTENING_NULLS_BASE);
        }
 }
 EXPORT_SYMBOL_GPL(inet_hashinfo_init);

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 7ba2d4f1de39097c050bedf591834db762e46c20..3c3b544404412f84faa68c46cb0e2a624cf4b4da 100644 (file)
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1945,13 +1945,14 @@ static void *listening_get_next(struct seq_file *seq, void *cur)
        struct tcp_iter_state *st = seq->private;
        struct net *net = seq_file_net(seq);
        struct inet_listen_hashbucket *ilb;
+       struct hlist_nulls_node *node;
        struct sock *sk = cur;
 
        if (!sk) {
 get_head:
                ilb = &tcp_hashinfo.listening_hash[st->bucket];
                spin_lock(&ilb->lock);
-               sk = sk_head(&ilb->head);
+               sk = sk_nulls_head(&ilb->nulls_head);
                st->offset = 0;
                goto get_sk;
        }
@@ -1959,9 +1960,9 @@ get_head:
        ++st->num;
        ++st->offset;
 
-       sk = sk_next(sk);
+       sk = sk_nulls_next(sk);
 get_sk:
-       sk_for_each_from(sk) {
+       sk_nulls_for_each_from(sk, node) {
                if (!net_eq(sock_net(sk), net))
                        continue;
                if (sk->sk_family == st->family)

diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index 228983a5531b994b0adad3a6d98b5d55c18a98aa..24a21979d7dff748f8bbe4c185eeea8f89e26400 100644 (file)
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -137,9 +137,10 @@ struct sock *inet6_lookup_listener(struct net *net,
        int score, hiscore = 0, matches = 0, reuseport = 0;
        bool exact_dif = inet6_exact_dif_match(net, skb);
        struct sock *sk, *result = NULL;
+       struct hlist_nulls_node *node;
        u32 phash = 0;
 
-       sk_for_each(sk, &ilb->head) {
+       sk_nulls_for_each(sk, node, &ilb->nulls_head) {
                score = compute_score(sk, net, hnum, daddr, dif, sdif, exact_dif);
                if (score > hiscore) {
                        reuseport = sk->sk_reuseport;