Linux kernel mirror (for testing)
git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel
os
linux
Merge branch 'net-avoid-ehash-lookup-races'
Xuanqiang Luo says:
====================
net: Avoid ehash lookup races
After replacing R/W locks with RCU in commit 3ab5aee7fe84 ("net: Convert
TCP & DCCP hash tables to use RCU / hlist_nulls"), a race window emerged
during the switch from reqsk/sk to sk/tw.
Now that both timewait sock (tw) and full sock (sk) reside on the same
ehash chain, it is appropriate to introduce hlist_nulls replace
operations, to eliminate the race conditions caused by this window.
Before this series of patches, I previously sent another version of the
patch, attempting to avoid the issue using a lock mechanism. However, it
seems there are some problems with that approach now, so I've switched to
the "replace" method in the current patches to resolve the issue.
For details, refer to:
https://lore.kernel.org/netdev/20250903024406.2418362-1-xuanqiang.luo@linux.dev/
Before I encountered this type of issue recently, I found there had been
several historical discussions about it. Therefore, I'm adding this
background information for those interested to reference:
1. https://lore.kernel.org/lkml/20230118015941.1313-1-kerneljasonxing@gmail.com/
2. https://lore.kernel.org/netdev/20230606064306.9192-1-duanmuquan@baidu.com/
====================
Link: https://patch.msgid.link/20251015020236.431822-1-xuanqiang.luo@linux.dev
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
+90
-25
+59
include/linux/rculist_nulls.h
+59
include/linux/rculist_nulls.h
···
53
53
(*((struct hlist_nulls_node __rcu __force **)&(node)->next))
54
54
55
55
/**
56
+
* hlist_nulls_pprev_rcu - returns the dereferenced pprev of @node.
57
+
* @node: element of the list.
58
+
*/
59
+
#define hlist_nulls_pprev_rcu(node) \
60
+
(*((struct hlist_nulls_node __rcu __force **)(node)->pprev))
61
+
62
+
/**
56
63
* hlist_nulls_del_rcu - deletes entry from hash list without re-initialization
57
64
* @n: the element to delete from the hash list.
58
65
*
···
157
150
{
158
151
n->pprev = &n->next;
159
152
n->next = (struct hlist_nulls_node *)NULLS_MARKER(NULL);
153
+
}
154
+
155
+
/**
156
+
* hlist_nulls_replace_rcu - replace an old entry by a new one
157
+
* @old: the element to be replaced
158
+
* @new: the new element to insert
159
+
*
160
+
* Description:
161
+
* Replace the old entry with the new one in a RCU-protected hlist_nulls, while
162
+
* permitting racing traversals.
163
+
*
164
+
* The caller must take whatever precautions are necessary (such as holding
165
+
* appropriate locks) to avoid racing with another list-mutation primitive, such
166
+
* as hlist_nulls_add_head_rcu() or hlist_nulls_del_rcu(), running on this same
167
+
* list. However, it is perfectly legal to run concurrently with the _rcu
168
+
* list-traversal primitives, such as hlist_nulls_for_each_entry_rcu().
169
+
*/
170
+
static inline void hlist_nulls_replace_rcu(struct hlist_nulls_node *old,
171
+
struct hlist_nulls_node *new)
172
+
{
173
+
struct hlist_nulls_node *next = old->next;
174
+
175
+
WRITE_ONCE(new->next, next);
176
+
WRITE_ONCE(new->pprev, old->pprev);
177
+
rcu_assign_pointer(hlist_nulls_pprev_rcu(new), new);
178
+
if (!is_a_nulls(next))
179
+
WRITE_ONCE(next->pprev, &new->next);
180
+
}
181
+
182
+
/**
183
+
* hlist_nulls_replace_init_rcu - replace an old entry by a new one and
184
+
* initialize the old
185
+
* @old: the element to be replaced
186
+
* @new: the new element to insert
187
+
*
188
+
* Description:
189
+
* Replace the old entry with the new one in a RCU-protected hlist_nulls, while
190
+
* permitting racing traversals, and reinitialize the old entry.
191
+
*
192
+
* Note: @old must be hashed.
193
+
*
194
+
* The caller must take whatever precautions are necessary (such as holding
195
+
* appropriate locks) to avoid racing with another list-mutation primitive, such
196
+
* as hlist_nulls_add_head_rcu() or hlist_nulls_del_rcu(), running on this same
197
+
* list. However, it is perfectly legal to run concurrently with the _rcu
198
+
* list-traversal primitives, such as hlist_nulls_for_each_entry_rcu().
199
+
*/
200
+
static inline void hlist_nulls_replace_init_rcu(struct hlist_nulls_node *old,
201
+
struct hlist_nulls_node *new)
202
+
{
203
+
hlist_nulls_replace_rcu(old, new);
204
+
WRITE_ONCE(old->pprev, NULL);
160
205
}
161
206
162
207
/**
+13
include/net/sock.h
+13
include/net/sock.h
···
856
856
return rc;
857
857
}
858
858
859
+
static inline bool sk_nulls_replace_node_init_rcu(struct sock *old,
860
+
struct sock *new)
861
+
{
862
+
if (sk_hashed(old)) {
863
+
hlist_nulls_replace_init_rcu(&old->sk_nulls_node,
864
+
&new->sk_nulls_node);
865
+
__sock_put(old);
866
+
return true;
867
+
}
868
+
869
+
return false;
870
+
}
871
+
859
872
static inline void __sk_add_node(struct sock *sk, struct hlist_head *list)
860
873
{
861
874
hlist_add_head(&sk->sk_node, list);
+6
-2
net/ipv4/inet_hashtables.c
+6
-2
net/ipv4/inet_hashtables.c
···
720
720
spin_lock(lock);
721
721
if (osk) {
722
722
WARN_ON_ONCE(sk->sk_hash != osk->sk_hash);
723
-
ret = sk_nulls_del_node_init_rcu(osk);
724
-
} else if (found_dup_sk) {
723
+
ret = sk_nulls_replace_node_init_rcu(osk, sk);
724
+
goto unlock;
725
+
}
726
+
727
+
if (found_dup_sk) {
725
728
*found_dup_sk = inet_ehash_lookup_by_sk(sk, list);
726
729
if (*found_dup_sk)
727
730
ret = false;
···
733
730
if (ret)
734
731
__sk_nulls_add_node_rcu(sk, list);
735
732
733
+
unlock:
736
734
spin_unlock(lock);
737
735
738
736
return ret;
+12
-23
net/ipv4/inet_timewait_sock.c
+12
-23
net/ipv4/inet_timewait_sock.c
···
88
88
}
89
89
EXPORT_SYMBOL_GPL(inet_twsk_put);
90
90
91
-
static void inet_twsk_add_node_rcu(struct inet_timewait_sock *tw,
92
-
struct hlist_nulls_head *list)
93
-
{
94
-
hlist_nulls_add_head_rcu(&tw->tw_node, list);
95
-
}
96
-
97
91
static void inet_twsk_schedule(struct inet_timewait_sock *tw, int timeo)
98
92
{
99
93
__inet_twsk_schedule(tw, timeo, false);
···
107
113
{
108
114
const struct inet_sock *inet = inet_sk(sk);
109
115
const struct inet_connection_sock *icsk = inet_csk(sk);
110
-
struct inet_ehash_bucket *ehead = inet_ehash_bucket(hashinfo, sk->sk_hash);
111
116
spinlock_t *lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
112
117
struct inet_bind_hashbucket *bhead, *bhead2;
113
118
114
-
/* Step 1: Put TW into bind hash. Original socket stays there too.
115
-
Note, that any socket with inet->num != 0 MUST be bound in
116
-
binding cache, even if it is closed.
119
+
/* Put TW into bind hash. Original socket stays there too.
120
+
* Note, that any socket with inet->num != 0 MUST be bound in
121
+
* binding cache, even if it is closed.
117
122
*/
118
123
bhead = &hashinfo->bhash[inet_bhashfn(twsk_net(tw), inet->inet_num,
119
124
hashinfo->bhash_size)];
···
134
141
135
142
spin_lock(lock);
136
143
137
-
/* Step 2: Hash TW into tcp ehash chain */
138
-
inet_twsk_add_node_rcu(tw, &ehead->chain);
139
-
140
-
/* Step 3: Remove SK from hash chain */
141
-
if (__sk_nulls_del_node_init_rcu(sk))
142
-
sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
143
-
144
-
145
-
/* Ensure above writes are committed into memory before updating the
146
-
* refcount.
147
-
* Provides ordering vs later refcount_inc().
148
-
*/
149
-
smp_wmb();
150
144
/* tw_refcnt is set to 3 because we have :
151
145
* - one reference for bhash chain.
152
146
* - one reference for ehash chain.
···
142
162
* so we are not allowed to use tw anymore.
143
163
*/
144
164
refcount_set(&tw->tw_refcnt, 3);
165
+
166
+
/* Ensure tw_refcnt has been set before tw is published.
167
+
* smp_wmb() provides the necessary memory barrier to enforce this
168
+
* ordering.
169
+
*/
170
+
smp_wmb();
171
+
172
+
hlist_nulls_replace_init_rcu(&sk->sk_nulls_node, &tw->tw_node);
173
+
sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
145
174
146
175
inet_twsk_schedule(tw, timeo);
147
176