* [PATCH net-next 00/10] udp: round of data-races fixes
@ 2023-09-12 9:17 Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 01/10] udp: introduce udp->udp_flags Eric Dumazet
` (11 more replies)
0 siblings, 12 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet
This series is inspired by multiple syzbot reports.
Many udp fields reads or writes are racy.
Add a proper udp->udp_flags and move there all
flags needing atomic safety.
Also add missing READ_ONCE()/WRITE_ONCE() when
lockless readers need access to specific fields.
Eric Dumazet (10):
udp: introduce udp->udp_flags
udp: move udp->no_check6_tx to udp->udp_flags
udp: move udp->no_check6_rx to udp->udp_flags
udp: move udp->gro_enabled to udp->udp_flags
udp: add missing WRITE_ONCE() around up->encap_rcv
udp: move udp->accept_udp_{l4|fraglist} to udp->udp_flags
udp: lockless UDP_ENCAP_L2TPINUDP / UDP_GRO
udp: annotate data-races around udp->encap_type
udplite: remove UDPLITE_BIT
udplite: fix various data-races
drivers/net/gtp.c | 4 +--
include/linux/udp.h | 66 ++++++++++++++++++++--------------
include/net/udp_tunnel.h | 9 ++---
include/net/udplite.h | 14 +++++---
net/ipv4/udp.c | 74 +++++++++++++++++++-------------------
net/ipv4/udp_offload.c | 4 +--
net/ipv4/udp_tunnel_core.c | 2 +-
net/ipv4/udplite.c | 1 -
net/ipv4/xfrm4_input.c | 4 +--
net/ipv6/udp.c | 34 +++++++++---------
net/ipv6/udplite.c | 1 -
net/ipv6/xfrm6_input.c | 4 +--
net/l2tp/l2tp_core.c | 6 ++--
13 files changed, 118 insertions(+), 105 deletions(-)
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next 01/10] udp: introduce udp->udp_flags
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
@ 2023-09-12 9:17 ` Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 02/10] udp: move udp->no_check6_tx to udp->udp_flags Eric Dumazet
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet
According to syzbot, it is time to use proper atomic flags
for various UDP flags.
Add udp_flags field, and convert udp->corkflag to first
bit in it.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/udp.h | 28 +++++++++++++++++++++-------
net/ipv4/udp.c | 12 ++++++------
net/ipv6/udp.c | 6 +++---
3 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index 43c1fb2d2c21afc01abdf20e4b9c03f04932c19b..23f0693e0d9cc4212ab7c7dc982561a2807b14f0 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -32,14 +32,20 @@ static inline u32 udp_hashfn(const struct net *net, u32 num, u32 mask)
return (num + net_hash_mix(net)) & mask;
}
+enum {
+ UDP_FLAGS_CORK, /* Cork is required */
+};
+
struct udp_sock {
/* inet_sock has to be the first member */
struct inet_sock inet;
#define udp_port_hash inet.sk.__sk_common.skc_u16hashes[0]
#define udp_portaddr_hash inet.sk.__sk_common.skc_u16hashes[1]
#define udp_portaddr_node inet.sk.__sk_common.skc_portaddr_node
+
+ unsigned long udp_flags;
+
int pending; /* Any pending frames ? */
- unsigned int corkflag; /* Cork is required */
__u8 encap_type; /* Is this an Encapsulation socket? */
unsigned char no_check6_tx:1,/* Send zero UDP6 checksums on TX? */
no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */
@@ -51,6 +57,11 @@ struct udp_sock {
gro_enabled:1, /* Request GRO aggregation */
accept_udp_l4:1,
accept_udp_fraglist:1;
+/* indicator bits used by pcflag: */
+#define UDPLITE_BIT 0x1 /* set by udplite proto init function */
+#define UDPLITE_SEND_CC 0x2 /* set via udplite setsockopt */
+#define UDPLITE_RECV_CC 0x4 /* set via udplite setsocktopt */
+ __u8 pcflag; /* marks socket as UDP-Lite if > 0 */
/*
* Following member retains the information to create a UDP header
* when the socket is uncorked.
@@ -62,12 +73,6 @@ struct udp_sock {
*/
__u16 pcslen;
__u16 pcrlen;
-/* indicator bits used by pcflag: */
-#define UDPLITE_BIT 0x1 /* set by udplite proto init function */
-#define UDPLITE_SEND_CC 0x2 /* set via udplite setsockopt */
-#define UDPLITE_RECV_CC 0x4 /* set via udplite setsocktopt */
- __u8 pcflag; /* marks socket as UDP-Lite if > 0 */
- __u8 unused[3];
/*
* For encapsulation sockets.
*/
@@ -95,6 +100,15 @@ struct udp_sock {
int forward_threshold;
};
+#define udp_test_bit(nr, sk) \
+ test_bit(UDP_FLAGS_##nr, &udp_sk(sk)->udp_flags)
+#define udp_set_bit(nr, sk) \
+ set_bit(UDP_FLAGS_##nr, &udp_sk(sk)->udp_flags)
+#define udp_clear_bit(nr, sk) \
+ clear_bit(UDP_FLAGS_##nr, &udp_sk(sk)->udp_flags)
+#define udp_assign_bit(nr, sk, val) \
+ assign_bit(UDP_FLAGS_##nr, &udp_sk(sk)->udp_flags, val)
+
#define UDP_MAX_SEGMENTS (1 << 6UL)
#define udp_sk(ptr) container_of_const(ptr, struct udp_sock, inet.sk)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index f39b9c8445808deee2c777cbb828474ff105d322..9709f8a532dc3d0e62a1cdb126ea7dc3cebd7da9 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1051,7 +1051,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
u8 tos, scope;
__be16 dport;
int err, is_udplite = IS_UDPLITE(sk);
- int corkreq = READ_ONCE(up->corkflag) || msg->msg_flags&MSG_MORE;
+ int corkreq = udp_test_bit(CORK, sk) || msg->msg_flags & MSG_MORE;
int (*getfrag)(void *, char *, int, int, int, struct sk_buff *);
struct sk_buff *skb;
struct ip_options_data opt_copy;
@@ -1315,11 +1315,11 @@ void udp_splice_eof(struct socket *sock)
struct sock *sk = sock->sk;
struct udp_sock *up = udp_sk(sk);
- if (!up->pending || READ_ONCE(up->corkflag))
+ if (!up->pending || udp_test_bit(CORK, sk))
return;
lock_sock(sk);
- if (up->pending && !READ_ONCE(up->corkflag))
+ if (up->pending && !udp_test_bit(CORK, sk))
udp_push_pending_frames(sk);
release_sock(sk);
}
@@ -2658,9 +2658,9 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
switch (optname) {
case UDP_CORK:
if (val != 0) {
- WRITE_ONCE(up->corkflag, 1);
+ udp_set_bit(CORK, sk);
} else {
- WRITE_ONCE(up->corkflag, 0);
+ udp_clear_bit(CORK, sk);
lock_sock(sk);
push_pending_frames(sk);
release_sock(sk);
@@ -2783,7 +2783,7 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
switch (optname) {
case UDP_CORK:
- val = READ_ONCE(up->corkflag);
+ val = udp_test_bit(CORK, sk);
break;
case UDP_ENCAP:
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 86b5d509a4688cacb2f40667c9ddc10f81ade2fe..0c6973cd22ce4cdcc846eb94029f4281e900da40 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1332,7 +1332,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
int addr_len = msg->msg_namelen;
bool connected = false;
int ulen = len;
- int corkreq = READ_ONCE(up->corkflag) || msg->msg_flags&MSG_MORE;
+ int corkreq = udp_test_bit(CORK, sk) || msg->msg_flags & MSG_MORE;
int err;
int is_udplite = IS_UDPLITE(sk);
int (*getfrag)(void *, char *, int, int, int, struct sk_buff *);
@@ -1644,11 +1644,11 @@ static void udpv6_splice_eof(struct socket *sock)
struct sock *sk = sock->sk;
struct udp_sock *up = udp_sk(sk);
- if (!up->pending || READ_ONCE(up->corkflag))
+ if (!up->pending || udp_test_bit(CORK, sk))
return;
lock_sock(sk);
- if (up->pending && !READ_ONCE(up->corkflag))
+ if (up->pending && !udp_test_bit(CORK, sk))
udp_v6_push_pending_frames(sk);
release_sock(sk);
}
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 02/10] udp: move udp->no_check6_tx to udp->udp_flags
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 01/10] udp: introduce udp->udp_flags Eric Dumazet
@ 2023-09-12 9:17 ` Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 03/10] udp: move udp->no_check6_rx " Eric Dumazet
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet, syzbot
syzbot reported that udp->no_check6_tx can be read locklessly.
Use one atomic bit from udp->udp_flags
Fixes: 1c19448c9ba6 ("net: Make enabling of zero UDP6 csums more restrictive")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/udp.h | 10 +++++-----
net/ipv4/udp.c | 4 ++--
net/ipv6/udp.c | 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index 23f0693e0d9cc4212ab7c7dc982561a2807b14f0..e3f2a6c7ac1d1bb37720420ce4cb0dc223926866 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -34,6 +34,7 @@ static inline u32 udp_hashfn(const struct net *net, u32 num, u32 mask)
enum {
UDP_FLAGS_CORK, /* Cork is required */
+ UDP_FLAGS_NO_CHECK6_TX, /* Send zero UDP6 checksums on TX? */
};
struct udp_sock {
@@ -47,8 +48,7 @@ struct udp_sock {
int pending; /* Any pending frames ? */
__u8 encap_type; /* Is this an Encapsulation socket? */
- unsigned char no_check6_tx:1,/* Send zero UDP6 checksums on TX? */
- no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */
+ unsigned char no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */
encap_enabled:1, /* This socket enabled encap
* processing; UDP tunnels and
* different encapsulation layer set
@@ -115,7 +115,7 @@ struct udp_sock {
static inline void udp_set_no_check6_tx(struct sock *sk, bool val)
{
- udp_sk(sk)->no_check6_tx = val;
+ udp_assign_bit(NO_CHECK6_TX, sk, val);
}
static inline void udp_set_no_check6_rx(struct sock *sk, bool val)
@@ -123,9 +123,9 @@ static inline void udp_set_no_check6_rx(struct sock *sk, bool val)
udp_sk(sk)->no_check6_rx = val;
}
-static inline bool udp_get_no_check6_tx(struct sock *sk)
+static inline bool udp_get_no_check6_tx(const struct sock *sk)
{
- return udp_sk(sk)->no_check6_tx;
+ return udp_test_bit(NO_CHECK6_TX, sk);
}
static inline bool udp_get_no_check6_rx(struct sock *sk)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 9709f8a532dc3d0e62a1cdb126ea7dc3cebd7da9..0c6998291c99d70d7d1f7e98af241642f75c1c22 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2694,7 +2694,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
break;
case UDP_NO_CHECK6_TX:
- up->no_check6_tx = valbool;
+ udp_set_no_check6_tx(sk, valbool);
break;
case UDP_NO_CHECK6_RX:
@@ -2791,7 +2791,7 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
break;
case UDP_NO_CHECK6_TX:
- val = up->no_check6_tx;
+ val = udp_get_no_check6_tx(sk);
break;
case UDP_NO_CHECK6_RX:
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 0c6973cd22ce4cdcc846eb94029f4281e900da40..469df0ca561f762f31deea1ca1836d49be7a9f3c 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1241,7 +1241,7 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
kfree_skb(skb);
return -EINVAL;
}
- if (udp_sk(sk)->no_check6_tx) {
+ if (udp_get_no_check6_tx(sk)) {
kfree_skb(skb);
return -EINVAL;
}
@@ -1262,7 +1262,7 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
if (is_udplite)
csum = udplite_csum(skb);
- else if (udp_sk(sk)->no_check6_tx) { /* UDP csum disabled */
+ else if (udp_get_no_check6_tx(sk)) { /* UDP csum disabled */
skb->ip_summed = CHECKSUM_NONE;
goto send;
} else if (skb->ip_summed == CHECKSUM_PARTIAL) { /* UDP hardware csum */
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 03/10] udp: move udp->no_check6_rx to udp->udp_flags
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 01/10] udp: introduce udp->udp_flags Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 02/10] udp: move udp->no_check6_tx to udp->udp_flags Eric Dumazet
@ 2023-09-12 9:17 ` Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 04/10] udp: move udp->gro_enabled " Eric Dumazet
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet, syzbot
syzbot reported that udp->no_check6_rx can be read locklessly.
Use one atomic bit from udp->udp_flags.
Fixes: 1c19448c9ba6 ("net: Make enabling of zero UDP6 csums more restrictive")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/udp.h | 10 +++++-----
net/ipv4/udp.c | 4 ++--
net/ipv6/udp.c | 6 +++---
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index e3f2a6c7ac1d1bb37720420ce4cb0dc223926866..8d4c3835b1b219da9c830f53f09aa0511840a1d4 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -35,6 +35,7 @@ static inline u32 udp_hashfn(const struct net *net, u32 num, u32 mask)
enum {
UDP_FLAGS_CORK, /* Cork is required */
UDP_FLAGS_NO_CHECK6_TX, /* Send zero UDP6 checksums on TX? */
+ UDP_FLAGS_NO_CHECK6_RX, /* Allow zero UDP6 checksums on RX? */
};
struct udp_sock {
@@ -48,8 +49,7 @@ struct udp_sock {
int pending; /* Any pending frames ? */
__u8 encap_type; /* Is this an Encapsulation socket? */
- unsigned char no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */
- encap_enabled:1, /* This socket enabled encap
+ unsigned char encap_enabled:1, /* This socket enabled encap
* processing; UDP tunnels and
* different encapsulation layer set
* this
@@ -120,7 +120,7 @@ static inline void udp_set_no_check6_tx(struct sock *sk, bool val)
static inline void udp_set_no_check6_rx(struct sock *sk, bool val)
{
- udp_sk(sk)->no_check6_rx = val;
+ udp_assign_bit(NO_CHECK6_RX, sk, val);
}
static inline bool udp_get_no_check6_tx(const struct sock *sk)
@@ -128,9 +128,9 @@ static inline bool udp_get_no_check6_tx(const struct sock *sk)
return udp_test_bit(NO_CHECK6_TX, sk);
}
-static inline bool udp_get_no_check6_rx(struct sock *sk)
+static inline bool udp_get_no_check6_rx(const struct sock *sk)
{
- return udp_sk(sk)->no_check6_rx;
+ return udp_test_bit(NO_CHECK6_RX, sk);
}
static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk,
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 0c6998291c99d70d7d1f7e98af241642f75c1c22..cb32826a1db20006ce9c21e66a9347f41b228da2 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2698,7 +2698,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
break;
case UDP_NO_CHECK6_RX:
- up->no_check6_rx = valbool;
+ udp_set_no_check6_rx(sk, valbool);
break;
case UDP_SEGMENT:
@@ -2795,7 +2795,7 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
break;
case UDP_NO_CHECK6_RX:
- val = up->no_check6_rx;
+ val = udp_get_no_check6_rx(sk);
break;
case UDP_SEGMENT:
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 469df0ca561f762f31deea1ca1836d49be7a9f3c..6e1ea3029260ec6e4992cf7e01c0202fabd94017 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -858,7 +858,7 @@ static int __udp6_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
/* If zero checksum and no_check is not on for
* the socket then skip it.
*/
- if (!uh->check && !udp_sk(sk)->no_check6_rx)
+ if (!uh->check && !udp_get_no_check6_rx(sk))
continue;
if (!first) {
first = sk;
@@ -980,7 +980,7 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
if (unlikely(rcu_dereference(sk->sk_rx_dst) != dst))
udp6_sk_rx_dst_set(sk, dst);
- if (!uh->check && !udp_sk(sk)->no_check6_rx) {
+ if (!uh->check && !udp_get_no_check6_rx(sk)) {
if (refcounted)
sock_put(sk);
goto report_csum_error;
@@ -1002,7 +1002,7 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
/* Unicast */
sk = __udp6_lib_lookup_skb(skb, uh->source, uh->dest, udptable);
if (sk) {
- if (!uh->check && !udp_sk(sk)->no_check6_rx)
+ if (!uh->check && !udp_get_no_check6_rx(sk))
goto report_csum_error;
return udp6_unicast_rcv_skb(sk, skb, uh);
}
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 04/10] udp: move udp->gro_enabled to udp->udp_flags
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
` (2 preceding siblings ...)
2023-09-12 9:17 ` [PATCH net-next 03/10] udp: move udp->no_check6_rx " Eric Dumazet
@ 2023-09-12 9:17 ` Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 05/10] udp: add missing WRITE_ONCE() around up->encap_rcv Eric Dumazet
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet, syzbot
syzbot reported that udp->gro_enabled can be read locklessly.
Use one atomic bit from udp->udp_flags.
Fixes: e20cf8d3f1f7 ("udp: implement GRO for plain UDP sockets.")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/udp.h | 2 +-
net/ipv4/udp.c | 6 +++---
net/ipv4/udp_offload.c | 4 ++--
net/ipv6/udp.c | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index 8d4c3835b1b219da9c830f53f09aa0511840a1d4..b344bd2e41fc9f4abf953c29c644b11438b0057d 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -36,6 +36,7 @@ enum {
UDP_FLAGS_CORK, /* Cork is required */
UDP_FLAGS_NO_CHECK6_TX, /* Send zero UDP6 checksums on TX? */
UDP_FLAGS_NO_CHECK6_RX, /* Allow zero UDP6 checksums on RX? */
+ UDP_FLAGS_GRO_ENABLED, /* Request GRO aggregation */
};
struct udp_sock {
@@ -54,7 +55,6 @@ struct udp_sock {
* different encapsulation layer set
* this
*/
- gro_enabled:1, /* Request GRO aggregation */
accept_udp_l4:1,
accept_udp_fraglist:1;
/* indicator bits used by pcflag: */
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index cb32826a1db20006ce9c21e66a9347f41b228da2..1debc10a0f029e47ffe90aaff60727b6bb7309cc 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1868,7 +1868,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int flags,
(struct sockaddr *)sin);
}
- if (udp_sk(sk)->gro_enabled)
+ if (udp_test_bit(GRO_ENABLED, sk))
udp_cmsg_recv(msg, sk, skb);
if (inet_cmsg_flags(inet))
@@ -2713,7 +2713,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
/* when enabling GRO, accept the related GSO packet type */
if (valbool)
udp_tunnel_encap_enable(sk->sk_socket);
- up->gro_enabled = valbool;
+ udp_assign_bit(GRO_ENABLED, sk, valbool);
up->accept_udp_l4 = valbool;
release_sock(sk);
break;
@@ -2803,7 +2803,7 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
break;
case UDP_GRO:
- val = up->gro_enabled;
+ val = udp_test_bit(GRO_ENABLED, sk);
break;
/* The following two cannot be changed on UDP sockets, the return is
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 0f46b3c2e4ac5427a5d39586086048afe22f34f1..6c95d28d0c4a7e56d587a986113b3711f8de964c 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -557,10 +557,10 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
NAPI_GRO_CB(skb)->is_flist = 0;
if (!sk || !udp_sk(sk)->gro_receive) {
if (skb->dev->features & NETIF_F_GRO_FRAGLIST)
- NAPI_GRO_CB(skb)->is_flist = sk ? !udp_sk(sk)->gro_enabled : 1;
+ NAPI_GRO_CB(skb)->is_flist = sk ? !udp_test_bit(GRO_ENABLED, sk) : 1;
if ((!sk && (skb->dev->features & NETIF_F_GRO_UDP_FWD)) ||
- (sk && udp_sk(sk)->gro_enabled) || NAPI_GRO_CB(skb)->is_flist)
+ (sk && udp_test_bit(GRO_ENABLED, sk)) || NAPI_GRO_CB(skb)->is_flist)
return call_gro_receive(udp_gro_receive_segment, head, skb);
/* no GRO, be sure flush the current packet */
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 6e1ea3029260ec6e4992cf7e01c0202fabd94017..2c3281879b6dfdf04bf219cf9b13c1c59f732a0d 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -413,7 +413,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
(struct sockaddr *)sin6);
}
- if (udp_sk(sk)->gro_enabled)
+ if (udp_test_bit(GRO_ENABLED, sk))
udp_cmsg_recv(msg, sk, skb);
if (np->rxopt.all)
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 05/10] udp: add missing WRITE_ONCE() around up->encap_rcv
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
` (3 preceding siblings ...)
2023-09-12 9:17 ` [PATCH net-next 04/10] udp: move udp->gro_enabled " Eric Dumazet
@ 2023-09-12 9:17 ` Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 06/10] udp: move udp->accept_udp_{l4|fraglist} to udp->udp_flags Eric Dumazet
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet
UDP_ENCAP_ESPINUDP_NON_IKE setsockopt() writes over up->encap_rcv
while other cpus read it.
Fixes: 067b207b281d ("[UDP]: Cleanup UDP encapsulation code")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/udp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 1debc10a0f029e47ffe90aaff60727b6bb7309cc..db43907b9a3e8d8f05c98e6a873415e6731261f4 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2675,10 +2675,12 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
case UDP_ENCAP_ESPINUDP_NON_IKE:
#if IS_ENABLED(CONFIG_IPV6)
if (sk->sk_family == AF_INET6)
- up->encap_rcv = ipv6_stub->xfrm6_udp_encap_rcv;
+ WRITE_ONCE(up->encap_rcv,
+ ipv6_stub->xfrm6_udp_encap_rcv);
else
#endif
- up->encap_rcv = xfrm4_udp_encap_rcv;
+ WRITE_ONCE(up->encap_rcv,
+ xfrm4_udp_encap_rcv);
#endif
fallthrough;
case UDP_ENCAP_L2TPINUDP:
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 06/10] udp: move udp->accept_udp_{l4|fraglist} to udp->udp_flags
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
` (4 preceding siblings ...)
2023-09-12 9:17 ` [PATCH net-next 05/10] udp: add missing WRITE_ONCE() around up->encap_rcv Eric Dumazet
@ 2023-09-12 9:17 ` Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 07/10] udp: lockless UDP_ENCAP_L2TPINUDP / UDP_GRO Eric Dumazet
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet
These are read locklessly, move them to udp_flags to fix data-races.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/udp.h | 16 +++++++++-------
net/ipv4/udp.c | 2 +-
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index b344bd2e41fc9f4abf953c29c644b11438b0057d..bb2b87adfbea9b8b8539a2d5a86f8f1208c84bff 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -37,6 +37,8 @@ enum {
UDP_FLAGS_NO_CHECK6_TX, /* Send zero UDP6 checksums on TX? */
UDP_FLAGS_NO_CHECK6_RX, /* Allow zero UDP6 checksums on RX? */
UDP_FLAGS_GRO_ENABLED, /* Request GRO aggregation */
+ UDP_FLAGS_ACCEPT_FRAGLIST,
+ UDP_FLAGS_ACCEPT_L4,
};
struct udp_sock {
@@ -50,13 +52,11 @@ struct udp_sock {
int pending; /* Any pending frames ? */
__u8 encap_type; /* Is this an Encapsulation socket? */
- unsigned char encap_enabled:1, /* This socket enabled encap
+ unsigned char encap_enabled:1; /* This socket enabled encap
* processing; UDP tunnels and
* different encapsulation layer set
* this
*/
- accept_udp_l4:1,
- accept_udp_fraglist:1;
/* indicator bits used by pcflag: */
#define UDPLITE_BIT 0x1 /* set by udplite proto init function */
#define UDPLITE_SEND_CC 0x2 /* set via udplite setsockopt */
@@ -149,10 +149,12 @@ static inline bool udp_unexpected_gso(struct sock *sk, struct sk_buff *skb)
if (!skb_is_gso(skb))
return false;
- if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 && !udp_sk(sk)->accept_udp_l4)
+ if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 &&
+ !udp_test_bit(ACCEPT_L4, sk))
return true;
- if (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST && !udp_sk(sk)->accept_udp_fraglist)
+ if (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST &&
+ !udp_test_bit(ACCEPT_FRAGLIST, sk))
return true;
return false;
@@ -160,8 +162,8 @@ static inline bool udp_unexpected_gso(struct sock *sk, struct sk_buff *skb)
static inline void udp_allow_gso(struct sock *sk)
{
- udp_sk(sk)->accept_udp_l4 = 1;
- udp_sk(sk)->accept_udp_fraglist = 1;
+ udp_set_bit(ACCEPT_L4, sk);
+ udp_set_bit(ACCEPT_FRAGLIST, sk);
}
#define udp_portaddr_for_each_entry(__sk, list) \
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index db43907b9a3e8d8f05c98e6a873415e6731261f4..75ba86a87bb6266f1054da0d631049ff279b21c7 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2716,7 +2716,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
if (valbool)
udp_tunnel_encap_enable(sk->sk_socket);
udp_assign_bit(GRO_ENABLED, sk, valbool);
- up->accept_udp_l4 = valbool;
+ udp_assign_bit(ACCEPT_L4, sk, valbool);
release_sock(sk);
break;
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 07/10] udp: lockless UDP_ENCAP_L2TPINUDP / UDP_GRO
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
` (5 preceding siblings ...)
2023-09-12 9:17 ` [PATCH net-next 06/10] udp: move udp->accept_udp_{l4|fraglist} to udp->udp_flags Eric Dumazet
@ 2023-09-12 9:17 ` Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 08/10] udp: annotate data-races around udp->encap_type Eric Dumazet
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet
Move udp->encap_enabled to udp->udp_flags.
Add udp_test_and_set_bit() helper to allow lockless
udp_tunnel_encap_enable() implementation.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/udp.h | 9 ++++-----
include/net/udp_tunnel.h | 9 +++------
net/ipv4/udp.c | 10 +++-------
net/ipv4/udp_tunnel_core.c | 2 +-
net/ipv6/udp.c | 2 +-
5 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index bb2b87adfbea9b8b8539a2d5a86f8f1208c84bff..0cf83270a4a28cfe726744bd6d1d1892978ae60a 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -39,6 +39,7 @@ enum {
UDP_FLAGS_GRO_ENABLED, /* Request GRO aggregation */
UDP_FLAGS_ACCEPT_FRAGLIST,
UDP_FLAGS_ACCEPT_L4,
+ UDP_FLAGS_ENCAP_ENABLED, /* This socket enabled encap */
};
struct udp_sock {
@@ -52,11 +53,7 @@ struct udp_sock {
int pending; /* Any pending frames ? */
__u8 encap_type; /* Is this an Encapsulation socket? */
- unsigned char encap_enabled:1; /* This socket enabled encap
- * processing; UDP tunnels and
- * different encapsulation layer set
- * this
- */
+
/* indicator bits used by pcflag: */
#define UDPLITE_BIT 0x1 /* set by udplite proto init function */
#define UDPLITE_SEND_CC 0x2 /* set via udplite setsockopt */
@@ -104,6 +101,8 @@ struct udp_sock {
test_bit(UDP_FLAGS_##nr, &udp_sk(sk)->udp_flags)
#define udp_set_bit(nr, sk) \
set_bit(UDP_FLAGS_##nr, &udp_sk(sk)->udp_flags)
+#define udp_test_and_set_bit(nr, sk) \
+ test_and_set_bit(UDP_FLAGS_##nr, &udp_sk(sk)->udp_flags)
#define udp_clear_bit(nr, sk) \
clear_bit(UDP_FLAGS_##nr, &udp_sk(sk)->udp_flags)
#define udp_assign_bit(nr, sk, val) \
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 0ca9b7a11baf5ccba18b4e3a7ef41e94c4ab2ab6..29251c3519cf0c73ed5512db57c495c0326b3549 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -174,16 +174,13 @@ static inline int udp_tunnel_handle_offloads(struct sk_buff *skb, bool udp_csum)
}
#endif
-static inline void udp_tunnel_encap_enable(struct socket *sock)
+static inline void udp_tunnel_encap_enable(struct sock *sk)
{
- struct udp_sock *up = udp_sk(sock->sk);
-
- if (up->encap_enabled)
+ if (udp_test_and_set_bit(ENCAP_ENABLED, sk))
return;
- up->encap_enabled = 1;
#if IS_ENABLED(CONFIG_IPV6)
- if (sock->sk->sk_family == PF_INET6)
+ if (READ_ONCE(sk->sk_family) == PF_INET6)
ipv6_stub->udpv6_encap_enable();
#endif
udp_encap_enable();
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 75ba86a87bb6266f1054da0d631049ff279b21c7..637a4faf9aff6389274bfb5ace1fc81dcb2db13f 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2618,7 +2618,7 @@ void udp_destroy_sock(struct sock *sk)
if (encap_destroy)
encap_destroy(sk);
}
- if (up->encap_enabled)
+ if (udp_test_bit(ENCAP_ENABLED, sk))
static_branch_dec(&udp_encap_needed_key);
}
}
@@ -2685,9 +2685,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
fallthrough;
case UDP_ENCAP_L2TPINUDP:
up->encap_type = val;
- lock_sock(sk);
- udp_tunnel_encap_enable(sk->sk_socket);
- release_sock(sk);
+ udp_tunnel_encap_enable(sk);
break;
default:
err = -ENOPROTOOPT;
@@ -2710,14 +2708,12 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
break;
case UDP_GRO:
- lock_sock(sk);
/* when enabling GRO, accept the related GSO packet type */
if (valbool)
- udp_tunnel_encap_enable(sk->sk_socket);
+ udp_tunnel_encap_enable(sk);
udp_assign_bit(GRO_ENABLED, sk, valbool);
udp_assign_bit(ACCEPT_L4, sk, valbool);
- release_sock(sk);
break;
/*
diff --git a/net/ipv4/udp_tunnel_core.c b/net/ipv4/udp_tunnel_core.c
index 9b18f371af0d49bd3ee9a440f222d03efd8a4911..1e7e4aecdc48a217a04544bb991bd1a8b8fe5ae4 100644
--- a/net/ipv4/udp_tunnel_core.c
+++ b/net/ipv4/udp_tunnel_core.c
@@ -78,7 +78,7 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
udp_sk(sk)->gro_receive = cfg->gro_receive;
udp_sk(sk)->gro_complete = cfg->gro_complete;
- udp_tunnel_encap_enable(sock);
+ udp_tunnel_encap_enable(sk);
}
EXPORT_SYMBOL_GPL(setup_udp_tunnel_sock);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 2c3281879b6dfdf04bf219cf9b13c1c59f732a0d..90688877e90049e45a164322119b2d411dd3e65e 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1670,7 +1670,7 @@ void udpv6_destroy_sock(struct sock *sk)
if (encap_destroy)
encap_destroy(sk);
}
- if (up->encap_enabled) {
+ if (udp_test_bit(ENCAP_ENABLED, sk)) {
static_branch_dec(&udpv6_encap_needed_key);
udp_encap_disable();
}
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 08/10] udp: annotate data-races around udp->encap_type
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
` (6 preceding siblings ...)
2023-09-12 9:17 ` [PATCH net-next 07/10] udp: lockless UDP_ENCAP_L2TPINUDP / UDP_GRO Eric Dumazet
@ 2023-09-12 9:17 ` Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 09/10] udplite: remove UDPLITE_BIT Eric Dumazet
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet, syzbot
syzbot/KCSAN complained about UDP_ENCAP_L2TPINUDP setsockopt() racing.
Add READ_ONCE()/WRITE_ONCE() to document races on this lockless field.
syzbot report was:
BUG: KCSAN: data-race in udp_lib_setsockopt / udp_lib_setsockopt
read-write to 0xffff8881083603fa of 1 bytes by task 16557 on cpu 0:
udp_lib_setsockopt+0x682/0x6c0
udp_setsockopt+0x73/0xa0 net/ipv4/udp.c:2779
sock_common_setsockopt+0x61/0x70 net/core/sock.c:3697
__sys_setsockopt+0x1c9/0x230 net/socket.c:2263
__do_sys_setsockopt net/socket.c:2274 [inline]
__se_sys_setsockopt net/socket.c:2271 [inline]
__x64_sys_setsockopt+0x66/0x80 net/socket.c:2271
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
read-write to 0xffff8881083603fa of 1 bytes by task 16554 on cpu 1:
udp_lib_setsockopt+0x682/0x6c0
udp_setsockopt+0x73/0xa0 net/ipv4/udp.c:2779
sock_common_setsockopt+0x61/0x70 net/core/sock.c:3697
__sys_setsockopt+0x1c9/0x230 net/socket.c:2263
__do_sys_setsockopt net/socket.c:2274 [inline]
__se_sys_setsockopt net/socket.c:2271 [inline]
__x64_sys_setsockopt+0x66/0x80 net/socket.c:2271
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
value changed: 0x01 -> 0x05
Reported by Kernel Concurrency Sanitizer on:
CPU: 1 PID: 16554 Comm: syz-executor.5 Not tainted 6.5.0-rc7-syzkaller-00004-gf7757129e3de #0
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
drivers/net/gtp.c | 4 ++--
net/ipv4/udp.c | 9 +++++----
net/ipv4/xfrm4_input.c | 4 ++--
net/ipv6/udp.c | 5 +++--
net/ipv6/xfrm6_input.c | 4 ++--
net/l2tp/l2tp_core.c | 6 +++---
6 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 144ec626230d6b0c4b60e9404d985aa569839309..b3aa0c3d58260c6b4b3cad09ac2678489f11e764 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -630,7 +630,7 @@ static void __gtp_encap_destroy(struct sock *sk)
gtp->sk0 = NULL;
else
gtp->sk1u = NULL;
- udp_sk(sk)->encap_type = 0;
+ WRITE_ONCE(udp_sk(sk)->encap_type, 0);
rcu_assign_sk_user_data(sk, NULL);
release_sock(sk);
sock_put(sk);
@@ -682,7 +682,7 @@ static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
netdev_dbg(gtp->dev, "encap_recv sk=%p\n", sk);
- switch (udp_sk(sk)->encap_type) {
+ switch (READ_ONCE(udp_sk(sk)->encap_type)) {
case UDP_ENCAP_GTP0:
netdev_dbg(gtp->dev, "received GTP0 packet\n");
ret = gtp0_udp_encap_recv(gtp, skb);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 637a4faf9aff6389274bfb5ace1fc81dcb2db13f..2eeab4af17a13de5a25a2da64259b8dc2fe3d047 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -714,7 +714,7 @@ int __udp4_lib_err(struct sk_buff *skb, u32 info, struct udp_table *udptable)
iph->saddr, uh->source, skb->dev->ifindex,
inet_sdif(skb), udptable, NULL);
- if (!sk || udp_sk(sk)->encap_type) {
+ if (!sk || READ_ONCE(udp_sk(sk)->encap_type)) {
/* No socket for error: try tunnels before discarding */
if (static_branch_unlikely(&udp_encap_needed_key)) {
sk = __udp4_lib_err_encap(net, iph, uh, udptable, sk, skb,
@@ -2081,7 +2081,8 @@ static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
}
nf_reset_ct(skb);
- if (static_branch_unlikely(&udp_encap_needed_key) && up->encap_type) {
+ if (static_branch_unlikely(&udp_encap_needed_key) &&
+ READ_ONCE(up->encap_type)) {
int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
/*
@@ -2684,7 +2685,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
#endif
fallthrough;
case UDP_ENCAP_L2TPINUDP:
- up->encap_type = val;
+ WRITE_ONCE(up->encap_type, val);
udp_tunnel_encap_enable(sk);
break;
default:
@@ -2785,7 +2786,7 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
break;
case UDP_ENCAP:
- val = up->encap_type;
+ val = READ_ONCE(up->encap_type);
break;
case UDP_NO_CHECK6_TX:
diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index eac206a290d05930c97f572e55ec0c26f3fefad0..183f6dc3724293f60c05aac630347405344c5010 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -85,11 +85,11 @@ int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb)
struct udphdr *uh;
struct iphdr *iph;
int iphlen, len;
-
__u8 *udpdata;
__be32 *udpdata32;
- __u16 encap_type = up->encap_type;
+ u16 encap_type;
+ encap_type = READ_ONCE(up->encap_type);
/* if this is not encapsulated socket, then just return now */
if (!encap_type)
return 1;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 90688877e90049e45a164322119b2d411dd3e65e..0e79d189613bef41e3ba39c463b19761d2fa3d34 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -571,7 +571,7 @@ int __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
sk = __udp6_lib_lookup(net, daddr, uh->dest, saddr, uh->source,
inet6_iif(skb), inet6_sdif(skb), udptable, NULL);
- if (!sk || udp_sk(sk)->encap_type) {
+ if (!sk || READ_ONCE(udp_sk(sk)->encap_type)) {
/* No socket for error: try tunnels before discarding */
if (static_branch_unlikely(&udpv6_encap_needed_key)) {
sk = __udp6_lib_err_encap(net, hdr, offset, uh,
@@ -688,7 +688,8 @@ static int udpv6_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
}
nf_reset_ct(skb);
- if (static_branch_unlikely(&udpv6_encap_needed_key) && up->encap_type) {
+ if (static_branch_unlikely(&udpv6_encap_needed_key) &&
+ READ_ONCE(up->encap_type)) {
int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
/*
diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
index 4907ab241d6bedf9ecfaa02017178e13fef4b980..4156387248e40e15b0dd10ccfa89cee5dd7e7534 100644
--- a/net/ipv6/xfrm6_input.c
+++ b/net/ipv6/xfrm6_input.c
@@ -81,14 +81,14 @@ int xfrm6_udp_encap_rcv(struct sock *sk, struct sk_buff *skb)
struct ipv6hdr *ip6h;
int len;
int ip6hlen = sizeof(struct ipv6hdr);
-
__u8 *udpdata;
__be32 *udpdata32;
- __u16 encap_type = up->encap_type;
+ u16 encap_type;
if (skb->protocol == htons(ETH_P_IP))
return xfrm4_udp_encap_rcv(sk, skb);
+ encap_type = READ_ONCE(up->encap_type);
/* if this is not encapsulated socket, then just return now */
if (!encap_type)
return 1;
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 03608d3ded4b83d1e59e064e482f54cffcdf5240..8d21ff25f1602f5389bed092cee92e9c4eebed3d 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1139,9 +1139,9 @@ static void l2tp_tunnel_destruct(struct sock *sk)
switch (tunnel->encap) {
case L2TP_ENCAPTYPE_UDP:
/* No longer an encapsulation socket. See net/ipv4/udp.c */
- (udp_sk(sk))->encap_type = 0;
- (udp_sk(sk))->encap_rcv = NULL;
- (udp_sk(sk))->encap_destroy = NULL;
+ WRITE_ONCE(udp_sk(sk)->encap_type, 0);
+ udp_sk(sk)->encap_rcv = NULL;
+ udp_sk(sk)->encap_destroy = NULL;
break;
case L2TP_ENCAPTYPE_IP:
break;
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 09/10] udplite: remove UDPLITE_BIT
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
` (7 preceding siblings ...)
2023-09-12 9:17 ` [PATCH net-next 08/10] udp: annotate data-races around udp->encap_type Eric Dumazet
@ 2023-09-12 9:17 ` Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 10/10] udplite: fix various data-races Eric Dumazet
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet
This flag is set but never read, we can remove it.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/udp.h | 5 ++---
net/ipv4/udplite.c | 1 -
net/ipv6/udplite.c | 1 -
3 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index 0cf83270a4a28cfe726744bd6d1d1892978ae60a..58156edec009636f2616b8a735945658d2982054 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -55,9 +55,8 @@ struct udp_sock {
__u8 encap_type; /* Is this an Encapsulation socket? */
/* indicator bits used by pcflag: */
-#define UDPLITE_BIT 0x1 /* set by udplite proto init function */
-#define UDPLITE_SEND_CC 0x2 /* set via udplite setsockopt */
-#define UDPLITE_RECV_CC 0x4 /* set via udplite setsocktopt */
+#define UDPLITE_SEND_CC 0x1 /* set via udplite setsockopt */
+#define UDPLITE_RECV_CC 0x2 /* set via udplite setsocktopt */
__u8 pcflag; /* marks socket as UDP-Lite if > 0 */
/*
* Following member retains the information to create a UDP header
diff --git a/net/ipv4/udplite.c b/net/ipv4/udplite.c
index 39ecdad1b50ce5608fa84b9d838c91c8704b3427..af37af3ab727bffeebe5c41d84cb7c130f49c50d 100644
--- a/net/ipv4/udplite.c
+++ b/net/ipv4/udplite.c
@@ -21,7 +21,6 @@ EXPORT_SYMBOL(udplite_table);
static int udplite_sk_init(struct sock *sk)
{
udp_init_sock(sk);
- udp_sk(sk)->pcflag = UDPLITE_BIT;
pr_warn_once("UDP-Lite is deprecated and scheduled to be removed in 2025, "
"please contact the netdev mailing list\n");
return 0;
diff --git a/net/ipv6/udplite.c b/net/ipv6/udplite.c
index 267d491e970753a1bb16babb8fbe85cd67cd7062..a60bec9b14f14a5b2d271f9965b5fca3d2a440c8 100644
--- a/net/ipv6/udplite.c
+++ b/net/ipv6/udplite.c
@@ -17,7 +17,6 @@
static int udplitev6_sk_init(struct sock *sk)
{
udpv6_init_sock(sk);
- udp_sk(sk)->pcflag = UDPLITE_BIT;
pr_warn_once("UDP-Lite is deprecated and scheduled to be removed in 2025, "
"please contact the netdev mailing list\n");
return 0;
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 10/10] udplite: fix various data-races
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
` (8 preceding siblings ...)
2023-09-12 9:17 ` [PATCH net-next 09/10] udplite: remove UDPLITE_BIT Eric Dumazet
@ 2023-09-12 9:17 ` Eric Dumazet
2023-09-12 13:25 ` [PATCH net-next 00/10] udp: round of data-races fixes Willem de Bruijn
2023-09-14 14:30 ` patchwork-bot+netdevbpf
11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-09-12 9:17 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet
udp->pcflag, udp->pcslen and udp->pcrlen reads/writes are racy.
Move udp->pcflag to udp->udp_flags for atomicity,
and add READ_ONCE()/WRITE_ONCE() annotations for pcslen and pcrlen.
Fixes: ba4e58eca8aa ("[NET]: Supporting UDP-Lite (RFC 3828) in Linux")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/udp.h | 6 ++----
include/net/udplite.h | 14 +++++++++-----
net/ipv4/udp.c | 21 +++++++++++----------
net/ipv6/udp.c | 9 +++++----
4 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index 58156edec009636f2616b8a735945658d2982054..d04188714dca14da25aa57083488cd28c34c41ba 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -40,6 +40,8 @@ enum {
UDP_FLAGS_ACCEPT_FRAGLIST,
UDP_FLAGS_ACCEPT_L4,
UDP_FLAGS_ENCAP_ENABLED, /* This socket enabled encap */
+ UDP_FLAGS_UDPLITE_SEND_CC, /* set via udplite setsockopt */
+ UDP_FLAGS_UDPLITE_RECV_CC, /* set via udplite setsockopt */
};
struct udp_sock {
@@ -54,10 +56,6 @@ struct udp_sock {
int pending; /* Any pending frames ? */
__u8 encap_type; /* Is this an Encapsulation socket? */
-/* indicator bits used by pcflag: */
-#define UDPLITE_SEND_CC 0x1 /* set via udplite setsockopt */
-#define UDPLITE_RECV_CC 0x2 /* set via udplite setsocktopt */
- __u8 pcflag; /* marks socket as UDP-Lite if > 0 */
/*
* Following member retains the information to create a UDP header
* when the socket is uncorked.
diff --git a/include/net/udplite.h b/include/net/udplite.h
index bd33ff2b8f426de24640cb7e821a28873a813260..786919d29f8de7664b5e8cabab00692ecf4b9089 100644
--- a/include/net/udplite.h
+++ b/include/net/udplite.h
@@ -66,14 +66,18 @@ static inline int udplite_checksum_init(struct sk_buff *skb, struct udphdr *uh)
/* Fast-path computation of checksum. Socket may not be locked. */
static inline __wsum udplite_csum(struct sk_buff *skb)
{
- const struct udp_sock *up = udp_sk(skb->sk);
const int off = skb_transport_offset(skb);
+ const struct sock *sk = skb->sk;
int len = skb->len - off;
- if ((up->pcflag & UDPLITE_SEND_CC) && up->pcslen < len) {
- if (0 < up->pcslen)
- len = up->pcslen;
- udp_hdr(skb)->len = htons(up->pcslen);
+ if (udp_test_bit(UDPLITE_SEND_CC, sk)) {
+ u16 pcslen = READ_ONCE(udp_sk(sk)->pcslen);
+
+ if (pcslen < len) {
+ if (pcslen > 0)
+ len = pcslen;
+ udp_hdr(skb)->len = htons(pcslen);
+ }
}
skb->ip_summed = CHECKSUM_NONE; /* no HW support for checksumming */
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 2eeab4af17a13de5a25a2da64259b8dc2fe3d047..c3ff984b63547daf0ecfb4ab96956aee2f8d589d 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2120,7 +2120,8 @@ static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
/*
* UDP-Lite specific tests, ignored on UDP sockets
*/
- if ((up->pcflag & UDPLITE_RECV_CC) && UDP_SKB_CB(skb)->partial_cov) {
+ if (udp_test_bit(UDPLITE_RECV_CC, sk) && UDP_SKB_CB(skb)->partial_cov) {
+ u16 pcrlen = READ_ONCE(up->pcrlen);
/*
* MIB statistics other than incrementing the error count are
@@ -2133,7 +2134,7 @@ static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
* delivery of packets with coverage values less than a value
* provided by the application."
*/
- if (up->pcrlen == 0) { /* full coverage was set */
+ if (pcrlen == 0) { /* full coverage was set */
net_dbg_ratelimited("UDPLite: partial coverage %d while full coverage %d requested\n",
UDP_SKB_CB(skb)->cscov, skb->len);
goto drop;
@@ -2144,9 +2145,9 @@ static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
* that it wants x while sender emits packets of smaller size y.
* Therefore the above ...()->partial_cov statement is essential.
*/
- if (UDP_SKB_CB(skb)->cscov < up->pcrlen) {
+ if (UDP_SKB_CB(skb)->cscov < pcrlen) {
net_dbg_ratelimited("UDPLite: coverage %d too small, need min %d\n",
- UDP_SKB_CB(skb)->cscov, up->pcrlen);
+ UDP_SKB_CB(skb)->cscov, pcrlen);
goto drop;
}
}
@@ -2729,8 +2730,8 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
val = 8;
else if (val > USHRT_MAX)
val = USHRT_MAX;
- up->pcslen = val;
- up->pcflag |= UDPLITE_SEND_CC;
+ WRITE_ONCE(up->pcslen, val);
+ udp_set_bit(UDPLITE_SEND_CC, sk);
break;
/* The receiver specifies a minimum checksum coverage value. To make
@@ -2743,8 +2744,8 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
val = 8;
else if (val > USHRT_MAX)
val = USHRT_MAX;
- up->pcrlen = val;
- up->pcflag |= UDPLITE_RECV_CC;
+ WRITE_ONCE(up->pcrlen, val);
+ udp_set_bit(UDPLITE_RECV_CC, sk);
break;
default:
@@ -2808,11 +2809,11 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
/* The following two cannot be changed on UDP sockets, the return is
* always 0 (which corresponds to the full checksum coverage of UDP). */
case UDPLITE_SEND_CSCOV:
- val = up->pcslen;
+ val = READ_ONCE(up->pcslen);
break;
case UDPLITE_RECV_CSCOV:
- val = up->pcrlen;
+ val = READ_ONCE(up->pcrlen);
break;
default:
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 0e79d189613bef41e3ba39c463b19761d2fa3d34..f60ba429543526b7ade2666c36dd51828ffe54a9 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -727,16 +727,17 @@ static int udpv6_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
/*
* UDP-Lite specific tests, ignored on UDP sockets (see net/ipv4/udp.c).
*/
- if ((up->pcflag & UDPLITE_RECV_CC) && UDP_SKB_CB(skb)->partial_cov) {
+ if (udp_test_bit(UDPLITE_RECV_CC, sk) && UDP_SKB_CB(skb)->partial_cov) {
+ u16 pcrlen = READ_ONCE(up->pcrlen);
- if (up->pcrlen == 0) { /* full coverage was set */
+ if (pcrlen == 0) { /* full coverage was set */
net_dbg_ratelimited("UDPLITE6: partial coverage %d while full coverage %d requested\n",
UDP_SKB_CB(skb)->cscov, skb->len);
goto drop;
}
- if (UDP_SKB_CB(skb)->cscov < up->pcrlen) {
+ if (UDP_SKB_CB(skb)->cscov < pcrlen) {
net_dbg_ratelimited("UDPLITE6: coverage %d too small, need min %d\n",
- UDP_SKB_CB(skb)->cscov, up->pcrlen);
+ UDP_SKB_CB(skb)->cscov, pcrlen);
goto drop;
}
}
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 00/10] udp: round of data-races fixes
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
` (9 preceding siblings ...)
2023-09-12 9:17 ` [PATCH net-next 10/10] udplite: fix various data-races Eric Dumazet
@ 2023-09-12 13:25 ` Willem de Bruijn
2023-09-14 14:30 ` patchwork-bot+netdevbpf
11 siblings, 0 replies; 13+ messages in thread
From: Willem de Bruijn @ 2023-09-12 13:25 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Willem de Bruijn, eric.dumazet, Eric Dumazet
Eric Dumazet wrote:
> This series is inspired by multiple syzbot reports.
>
> Many udp fields reads or writes are racy.
>
> Add a proper udp->udp_flags and move there all
> flags needing atomic safety.
>
> Also add missing READ_ONCE()/WRITE_ONCE() when
> lockless readers need access to specific fields.
>
> Eric Dumazet (10):
> udp: introduce udp->udp_flags
> udp: move udp->no_check6_tx to udp->udp_flags
> udp: move udp->no_check6_rx to udp->udp_flags
> udp: move udp->gro_enabled to udp->udp_flags
> udp: add missing WRITE_ONCE() around up->encap_rcv
> udp: move udp->accept_udp_{l4|fraglist} to udp->udp_flags
> udp: lockless UDP_ENCAP_L2TPINUDP / UDP_GRO
> udp: annotate data-races around udp->encap_type
> udplite: remove UDPLITE_BIT
> udplite: fix various data-races
>
> drivers/net/gtp.c | 4 +--
> include/linux/udp.h | 66 ++++++++++++++++++++--------------
> include/net/udp_tunnel.h | 9 ++---
> include/net/udplite.h | 14 +++++---
> net/ipv4/udp.c | 74 +++++++++++++++++++-------------------
> net/ipv4/udp_offload.c | 4 +--
> net/ipv4/udp_tunnel_core.c | 2 +-
> net/ipv4/udplite.c | 1 -
> net/ipv4/xfrm4_input.c | 4 +--
> net/ipv6/udp.c | 34 +++++++++---------
> net/ipv6/udplite.c | 1 -
> net/ipv6/xfrm6_input.c | 4 +--
> net/l2tp/l2tp_core.c | 6 ++--
> 13 files changed, 118 insertions(+), 105 deletions(-)
For the series:
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 00/10] udp: round of data-races fixes
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
` (10 preceding siblings ...)
2023-09-12 13:25 ` [PATCH net-next 00/10] udp: round of data-races fixes Willem de Bruijn
@ 2023-09-14 14:30 ` patchwork-bot+netdevbpf
11 siblings, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-09-14 14:30 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuba, pabeni, netdev, willemdebruijn.kernel, eric.dumazet
Hello:
This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 12 Sep 2023 09:17:20 +0000 you wrote:
> This series is inspired by multiple syzbot reports.
>
> Many udp fields reads or writes are racy.
>
> Add a proper udp->udp_flags and move there all
> flags needing atomic safety.
>
> [...]
Here is the summary with links:
- [net-next,01/10] udp: introduce udp->udp_flags
https://git.kernel.org/netdev/net-next/c/81b36803ac13
- [net-next,02/10] udp: move udp->no_check6_tx to udp->udp_flags
https://git.kernel.org/netdev/net-next/c/a0002127cd74
- [net-next,03/10] udp: move udp->no_check6_rx to udp->udp_flags
https://git.kernel.org/netdev/net-next/c/bcbc1b1de884
- [net-next,04/10] udp: move udp->gro_enabled to udp->udp_flags
https://git.kernel.org/netdev/net-next/c/e1dc0615c6b0
- [net-next,05/10] udp: add missing WRITE_ONCE() around up->encap_rcv
https://git.kernel.org/netdev/net-next/c/6d5a12eb9122
- [net-next,06/10] udp: move udp->accept_udp_{l4|fraglist} to udp->udp_flags
https://git.kernel.org/netdev/net-next/c/f5f52f0884a5
- [net-next,07/10] udp: lockless UDP_ENCAP_L2TPINUDP / UDP_GRO
https://git.kernel.org/netdev/net-next/c/ac9a7f4ce5dd
- [net-next,08/10] udp: annotate data-races around udp->encap_type
https://git.kernel.org/netdev/net-next/c/70a36f571362
- [net-next,09/10] udplite: remove UDPLITE_BIT
https://git.kernel.org/netdev/net-next/c/729549aa350c
- [net-next,10/10] udplite: fix various data-races
https://git.kernel.org/netdev/net-next/c/882af43a0fc3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-09-14 14:30 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-12 9:17 [PATCH net-next 00/10] udp: round of data-races fixes Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 01/10] udp: introduce udp->udp_flags Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 02/10] udp: move udp->no_check6_tx to udp->udp_flags Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 03/10] udp: move udp->no_check6_rx " Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 04/10] udp: move udp->gro_enabled " Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 05/10] udp: add missing WRITE_ONCE() around up->encap_rcv Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 06/10] udp: move udp->accept_udp_{l4|fraglist} to udp->udp_flags Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 07/10] udp: lockless UDP_ENCAP_L2TPINUDP / UDP_GRO Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 08/10] udp: annotate data-races around udp->encap_type Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 09/10] udplite: remove UDPLITE_BIT Eric Dumazet
2023-09-12 9:17 ` [PATCH net-next 10/10] udplite: fix various data-races Eric Dumazet
2023-09-12 13:25 ` [PATCH net-next 00/10] udp: round of data-races fixes Willem de Bruijn
2023-09-14 14:30 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).