Skip to content

Commit 621a3b0

Browse files
author
Herton R. Krzesinski
committed
Merge: net: Backport data race annotations in the networking stack (part 1).
MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/1722 Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2149949 Upstream Status: linux.git Conflicts: Few minor conflicts, see description in affected commits. Properly mark concurent reads and writes with READ_ONCE() and WRITE_ONCE() in various parts of the networking stack. This is a backport of the following upstream patch series: * Patch set A: merge commit e97e68b ("Merge branch 'sk_bound_dev_if-annotations'") * Patch set B: merge commit 32b3ad1 ("Merge branch 'sysctl-data-races'") * Patch set C: merge commit 7d5424b ("Merge branch 'net-sysctl-races'") * Patch set D: merge commit 782d86f ("Merge branch 'net-sysctl-races-round2'") * Patch set E: merge commit c9f2110 ("Merge branch 'net-ipv4-sysctl-races-part-3'") Patch 1 is a standalone READ_ONCE() annotation for sk->sk_bound_dev_if. It's a prerequisite for correctly backporting patch set A. Patches 2-9 are backports of patch set A. The following upstream patches have been omitted since they're already in Centos Stream: * Upstream commit a20ea29 ("sctp: read sk->sk_bound_dev_if once in sctp_rcv()"), backported by Centos Stream commit 5d539b8. * Upstream commit 70f87de ("net_sched: em_meta: add READ_ONCE() in var_sk_bound_if()"), backported by Centos Stream commit 866ca28. Patch 10 was in the original upstream series of patch set B, but was resubmitted independently as that series was reworked before being applied. Therefore, it doesn't strictly belong to patch set B, but is closely related to it and is thus backported here. Patches 11-21 are backports of patch set B. The following upstream patch has been omitted since it's already in Centos Stream: * Upstream commit 310731e ("net: Fix data-races around sysctl_mem.", backported by Centos Stream commit a99b2cb. Patches 22-36 are backports corresponding to patch set C. Patches 37-51 are backports corresponding to patch set D. Patches 52-66 are backports corresponding to patch set E. Signed-off-by: Guillaume Nault <gnault@redhat.com> Approved-by: Antoine Tenart <atenart@redhat.com> Approved-by: Michal Schmidt <mschmidt@redhat.com> Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
2 parents e0197de + d3f4418 commit 621a3b0

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+290
-220
lines changed

Documentation/networking/ip-sysctl.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,7 +1066,7 @@ cipso_cache_enable - BOOLEAN
10661066
cipso_cache_bucket_size - INTEGER
10671067
The CIPSO label cache consists of a fixed size hash table with each
10681068
hash bucket containing a number of cache entries. This variable limits
1069-
the number of entries in each hash bucket; the larger the value the
1069+
the number of entries in each hash bucket; the larger the value is, the
10701070
more CIPSO label mappings that can be cached. When the number of
10711071
entries in a given hash bucket reaches this limit adding new entries
10721072
causes the oldest entry in the bucket to be removed to make room.
@@ -1160,7 +1160,7 @@ ip_autobind_reuse - BOOLEAN
11601160
option should only be set by experts.
11611161
Default: 0
11621162

1163-
ip_dynaddr - BOOLEAN
1163+
ip_dynaddr - INTEGER
11641164
If set non-zero, enables support for dynamic addresses.
11651165
If set to a non-zero value larger than 1, a kernel log
11661166
message will be printed when dynamic address rewriting

drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1391,7 +1391,7 @@ static void chtls_pass_accept_request(struct sock *sk,
13911391
th_ecn = tcph->ece && tcph->cwr;
13921392
if (th_ecn) {
13931393
ect = !INET_ECN_is_not_ect(ip_dsfield);
1394-
ecn_ok = sock_net(sk)->ipv4.sysctl_tcp_ecn;
1394+
ecn_ok = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_ecn);
13951395
if ((!ect && ecn_ok) || tcp_ca_needs_ecn(sk))
13961396
inet_rsk(oreq)->ecn_ok = 1;
13971397
}

drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10224,13 +10224,14 @@ static int mlxsw_sp_dscp_init(struct mlxsw_sp *mlxsw_sp)
1022410224
static int __mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
1022510225
{
1022610226
struct net *net = mlxsw_sp_net(mlxsw_sp);
10227-
bool usp = net->ipv4.sysctl_ip_fwd_update_priority;
1022810227
char rgcr_pl[MLXSW_REG_RGCR_LEN];
1022910228
u64 max_rifs;
10229+
bool usp;
1023010230

1023110231
if (!MLXSW_CORE_RES_VALID(mlxsw_sp->core, MAX_RIFS))
1023210232
return -EIO;
1023310233
max_rifs = MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_RIFS);
10234+
usp = READ_ONCE(net->ipv4.sysctl_ip_fwd_update_priority);
1023410235

1023510236
mlxsw_reg_rgcr_pack(rgcr_pl, true, true);
1023610237
mlxsw_reg_rgcr_max_router_interfaces_set(rgcr_pl, max_rifs);

include/net/inet6_hashtables.h

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,25 @@ struct sock *inet6_lookup(struct net *net, struct inet_hashinfo *hashinfo,
103103
const int dif);
104104

105105
int inet6_hash(struct sock *sk);
106-
#endif /* IS_ENABLED(CONFIG_IPV6) */
107106

108-
#define INET6_MATCH(__sk, __net, __saddr, __daddr, __ports, __dif, __sdif) \
109-
(((__sk)->sk_portpair == (__ports)) && \
110-
((__sk)->sk_family == AF_INET6) && \
111-
ipv6_addr_equal(&(__sk)->sk_v6_daddr, (__saddr)) && \
112-
ipv6_addr_equal(&(__sk)->sk_v6_rcv_saddr, (__daddr)) && \
113-
(((__sk)->sk_bound_dev_if == (__dif)) || \
114-
((__sk)->sk_bound_dev_if == (__sdif))) && \
115-
net_eq(sock_net(__sk), (__net)))
107+
static inline bool inet6_match(struct net *net, const struct sock *sk,
108+
const struct in6_addr *saddr,
109+
const struct in6_addr *daddr,
110+
const __portpair ports,
111+
const int dif, const int sdif)
112+
{
113+
int bound_dev_if;
114+
115+
if (!net_eq(sock_net(sk), net) ||
116+
sk->sk_family != AF_INET6 ||
117+
sk->sk_portpair != ports ||
118+
!ipv6_addr_equal(&sk->sk_v6_daddr, saddr) ||
119+
!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
120+
return false;
121+
122+
bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
123+
return bound_dev_if == dif || bound_dev_if == sdif;
124+
}
125+
#endif /* IS_ENABLED(CONFIG_IPV6) */
116126

117127
#endif /* _INET6_HASHTABLES_H */

include/net/inet_hashtables.h

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ static inline bool inet_sk_bound_dev_eq(struct net *net, int bound_dev_if,
179179
int dif, int sdif)
180180
{
181181
#if IS_ENABLED(CONFIG_NET_L3_MASTER_DEV)
182-
return inet_bound_dev_eq(!!net->ipv4.sysctl_tcp_l3mdev_accept,
182+
return inet_bound_dev_eq(!!READ_ONCE(net->ipv4.sysctl_tcp_l3mdev_accept),
183183
bound_dev_if, dif, sdif);
184184
#else
185185
return inet_bound_dev_eq(true, bound_dev_if, dif, sdif);
@@ -255,7 +255,6 @@ static inline struct sock *inet_lookup_listener(struct net *net,
255255
((__force __portpair)(((__u32)(__dport) << 16) | (__force __u32)(__be16)(__sport)))
256256
#endif
257257

258-
#if (BITS_PER_LONG == 64)
259258
#ifdef __BIG_ENDIAN
260259
#define INET_ADDR_COOKIE(__name, __saddr, __daddr) \
261260
const __addrpair __name = (__force __addrpair) ( \
@@ -267,24 +266,22 @@ static inline struct sock *inet_lookup_listener(struct net *net,
267266
(((__force __u64)(__be32)(__daddr)) << 32) | \
268267
((__force __u64)(__be32)(__saddr)))
269268
#endif /* __BIG_ENDIAN */
270-
#define INET_MATCH(__sk, __net, __cookie, __saddr, __daddr, __ports, __dif, __sdif) \
271-
(((__sk)->sk_portpair == (__ports)) && \
272-
((__sk)->sk_addrpair == (__cookie)) && \
273-
(((__sk)->sk_bound_dev_if == (__dif)) || \
274-
((__sk)->sk_bound_dev_if == (__sdif))) && \
275-
net_eq(sock_net(__sk), (__net)))
276-
#else /* 32-bit arch */
277-
#define INET_ADDR_COOKIE(__name, __saddr, __daddr) \
278-
const int __name __deprecated __attribute__((unused))
279-
280-
#define INET_MATCH(__sk, __net, __cookie, __saddr, __daddr, __ports, __dif, __sdif) \
281-
(((__sk)->sk_portpair == (__ports)) && \
282-
((__sk)->sk_daddr == (__saddr)) && \
283-
((__sk)->sk_rcv_saddr == (__daddr)) && \
284-
(((__sk)->sk_bound_dev_if == (__dif)) || \
285-
((__sk)->sk_bound_dev_if == (__sdif))) && \
286-
net_eq(sock_net(__sk), (__net)))
287-
#endif /* 64-bit arch */
269+
270+
static inline bool inet_match(struct net *net, const struct sock *sk,
271+
const __addrpair cookie, const __portpair ports,
272+
int dif, int sdif)
273+
{
274+
int bound_dev_if;
275+
276+
if (!net_eq(sock_net(sk), net) ||
277+
sk->sk_portpair != ports ||
278+
sk->sk_addrpair != cookie)
279+
return false;
280+
281+
/* Paired with WRITE_ONCE() from sock_bindtoindex_locked() */
282+
bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
283+
return bound_dev_if == dif || bound_dev_if == sdif;
284+
}
288285

289286
/* Sockets in TCP_CLOSE state are _always_ taken out of the hash, so we need
290287
* not check it for lookups anymore, thanks Alexey. -DaveM

include/net/inet_sock.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ static inline struct inet_request_sock *inet_rsk(const struct request_sock *sk)
107107

108108
static inline u32 inet_request_mark(const struct sock *sk, struct sk_buff *skb)
109109
{
110-
if (!sk->sk_mark && sock_net(sk)->ipv4.sysctl_tcp_fwmark_accept)
110+
if (!sk->sk_mark &&
111+
READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_fwmark_accept))
111112
return skb->mark;
112113

113114
return sk->sk_mark;
@@ -116,22 +117,23 @@ static inline u32 inet_request_mark(const struct sock *sk, struct sk_buff *skb)
116117
static inline int inet_request_bound_dev_if(const struct sock *sk,
117118
struct sk_buff *skb)
118119
{
120+
int bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
119121
#ifdef CONFIG_NET_L3_MASTER_DEV
120122
struct net *net = sock_net(sk);
121123

122-
if (!sk->sk_bound_dev_if && net->ipv4.sysctl_tcp_l3mdev_accept)
124+
if (!bound_dev_if && READ_ONCE(net->ipv4.sysctl_tcp_l3mdev_accept))
123125
return l3mdev_master_ifindex_by_index(net, skb->skb_iif);
124126
#endif
125127

126-
return sk->sk_bound_dev_if;
128+
return bound_dev_if;
127129
}
128130

129131
static inline int inet_sk_bound_l3mdev(const struct sock *sk)
130132
{
131133
#ifdef CONFIG_NET_L3_MASTER_DEV
132134
struct net *net = sock_net(sk);
133135

134-
if (!net->ipv4.sysctl_tcp_l3mdev_accept)
136+
if (!READ_ONCE(net->ipv4.sysctl_tcp_l3mdev_accept))
135137
return l3mdev_master_ifindex_by_index(net,
136138
sk->sk_bound_dev_if);
137139
#endif
@@ -373,7 +375,7 @@ static inline bool inet_get_convert_csum(struct sock *sk)
373375
static inline bool inet_can_nonlocal_bind(struct net *net,
374376
struct inet_sock *inet)
375377
{
376-
return net->ipv4.sysctl_ip_nonlocal_bind ||
378+
return READ_ONCE(net->ipv4.sysctl_ip_nonlocal_bind) ||
377379
inet->freebind || inet->transparent;
378380
}
379381

include/net/ip.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ static inline void ipcm_init_sk(struct ipcm_cookie *ipcm,
9292

9393
ipcm->sockc.mark = inet->sk.sk_mark;
9494
ipcm->sockc.tsflags = inet->sk.sk_tsflags;
95-
ipcm->oif = inet->sk.sk_bound_dev_if;
95+
ipcm->oif = READ_ONCE(inet->sk.sk_bound_dev_if);
9696
ipcm->addr = inet->inet_saddr;
9797
}
9898

@@ -382,7 +382,7 @@ void ipfrag_init(void);
382382
void ip_static_sysctl_init(void);
383383

384384
#define IP4_REPLY_MARK(net, mark) \
385-
((net)->ipv4.sysctl_fwmark_reflect ? (mark) : 0)
385+
(READ_ONCE((net)->ipv4.sysctl_fwmark_reflect) ? (mark) : 0)
386386

387387
static inline bool ip_is_fragment(const struct iphdr *iph)
388388
{
@@ -443,7 +443,7 @@ static inline unsigned int ip_dst_mtu_maybe_forward(const struct dst_entry *dst,
443443
struct net *net = dev_net(dst->dev);
444444
unsigned int mtu;
445445

446-
if (net->ipv4.sysctl_ip_fwd_use_pmtu ||
446+
if (READ_ONCE(net->ipv4.sysctl_ip_fwd_use_pmtu) ||
447447
ip_mtu_locked(dst) ||
448448
!forwarding)
449449
return dst_mtu(dst);

include/net/raw.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ static inline bool raw_sk_bound_dev_eq(struct net *net, int bound_dev_if,
8383
int dif, int sdif)
8484
{
8585
#if IS_ENABLED(CONFIG_NET_L3_MASTER_DEV)
86-
return inet_bound_dev_eq(!!net->ipv4.sysctl_raw_l3mdev_accept,
86+
return inet_bound_dev_eq(READ_ONCE(net->ipv4.sysctl_raw_l3mdev_accept),
8787
bound_dev_if, dif, sdif);
8888
#else
8989
return inet_bound_dev_eq(true, bound_dev_if, dif, sdif);

include/net/route.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ static inline int ip4_dst_hoplimit(const struct dst_entry *dst)
361361
struct net *net = dev_net(dst->dev);
362362

363363
if (hoplimit == 0)
364-
hoplimit = net->ipv4.sysctl_ip_default_ttl;
364+
hoplimit = READ_ONCE(net->ipv4.sysctl_ip_default_ttl);
365365
return hoplimit;
366366
}
367367

include/net/sock.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,6 @@ typedef __u64 __bitwise __addrpair;
160160
* for struct sock and struct inet_timewait_sock.
161161
*/
162162
struct sock_common {
163-
/* skc_daddr and skc_rcv_saddr must be grouped on a 8 bytes aligned
164-
* address on 64bit arches : cf INET_MATCH()
165-
*/
166163
union {
167164
__addrpair skc_addrpair;
168165
struct {
@@ -2851,13 +2848,14 @@ static inline void sk_pacing_shift_update(struct sock *sk, int val)
28512848
*/
28522849
static inline bool sk_dev_equal_l3scope(struct sock *sk, int dif)
28532850
{
2851+
int bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
28542852
int mdif;
28552853

2856-
if (!sk->sk_bound_dev_if || sk->sk_bound_dev_if == dif)
2854+
if (!bound_dev_if || bound_dev_if == dif)
28572855
return true;
28582856

28592857
mdif = l3mdev_master_ifindex_by_index(sock_net(sk), dif);
2860-
if (mdif && mdif == sk->sk_bound_dev_if)
2858+
if (mdif && mdif == bound_dev_if)
28612859
return true;
28622860

28632861
return false;

0 commit comments

Comments
 (0)