From 302fd47008c3206fafe08fd3beba73076563ab04 Mon Sep 17 00:00:00 2001 From: liuchun Date: Wed, 2 Nov 2022 14:48:48 +0800 Subject: [PATCH] raw: convert raw sockets to RCU Upstream commit:0daf07e527095e64ee8927ce297ab626643e9f51 Upstream commit:af185d8c76333daa877678e0166a7b45e63bf3c4 Using rwlock in networking code is extremely risky. writers can starve if enough readers are constantly grabing the rwlock. I thought rwlock were at fault and sent this patch: https://lkml.org/lkml/2022/6/17/272 But Peter and Linus essentially told me rwlock had to be unfair. We need to get rid of rwlock in networking code. Without this fix, following script triggers soft lockups: for i in {1..48} do ping -f -n -q 127.0.0.1 & sleep 0.1 done Conflicts: rewrite net/ipv4/raw.c net/ipv4/raw_diag.c net/ipv6/raw.c Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller Link: https://lore.kernel.org/r/20220620100509.3493504-1-eric.dumazet@gmail.com Signed-off-by: Paolo Abeni Signed-off-by: Chun Liu --- include/net/raw.h | 13 +++++++-- include/net/rawv6.h | 1 + net/ipv4/af_inet.c | 2 ++ net/ipv4/raw.c | 71 ++++++++++++++++++++++----------------------- net/ipv4/raw_diag.c | 22 +++++++++----- net/ipv6/af_inet6.c | 3 ++ net/ipv6/raw.c | 25 ++++++++-------- 7 files changed, 77 insertions(+), 60 deletions(-) diff --git a/include/net/raw.h b/include/net/raw.h index 8ad8df594..9f11c2950 100644 --- a/include/net/raw.h +++ b/include/net/raw.h @@ -33,10 +33,19 @@ int raw_rcv(struct sock *, struct sk_buff *); #define RAW_HTABLE_SIZE MAX_INET_PROTOS struct raw_hashinfo { - rwlock_t lock; - struct hlist_head ht[RAW_HTABLE_SIZE]; + spinlock_t lock; + struct hlist_nulls_head ht[RAW_HTABLE_SIZE]; }; +static inline void raw_hashinfo_init(struct raw_hashinfo *hashinfo) +{ + int i; + + spin_lock_init(&hashinfo->lock); + for (i = 0; i < RAW_HTABLE_SIZE; i++) + INIT_HLIST_NULLS_HEAD(&hashinfo->ht[i], i); +} + #ifdef CONFIG_PROC_FS int raw_proc_init(void); void raw_proc_exit(void); diff --git a/include/net/rawv6.h b/include/net/rawv6.h index 53d86b605..2efa4a886 100644 --- a/include/net/rawv6.h +++ b/include/net/rawv6.h @@ -3,6 +3,7 @@ #define _NET_RAWV6_H #include +#include extern struct raw_hashinfo raw_v6_hashinfo; struct sock *__raw_v6_lookup(struct net *net, struct sock *sk, diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index a3e842036..5226b0aeb 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1927,6 +1927,8 @@ static int __init inet_init(void) sock_skb_cb_check_size(sizeof(struct inet_skb_parm)); + raw_hashinfo_init(&raw_v4_hashinfo); + rc = proto_register(&tcp_prot, 1); if (rc) goto out; diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c index 3183413eb..bb3665441 100644 --- a/net/ipv4/raw.c +++ b/net/ipv4/raw.c @@ -85,22 +85,21 @@ struct raw_frag_vec { int hlen; }; -struct raw_hashinfo raw_v4_hashinfo = { - .lock = __RW_LOCK_UNLOCKED(raw_v4_hashinfo.lock), -}; +struct raw_hashinfo raw_v4_hashinfo; EXPORT_SYMBOL_GPL(raw_v4_hashinfo); int raw_hash_sk(struct sock *sk) { struct raw_hashinfo *h = sk->sk_prot->h.raw_hash; - struct hlist_head *head; + struct hlist_nulls_head *hlist; - head = &h->ht[inet_sk(sk)->inet_num & (RAW_HTABLE_SIZE - 1)]; + hlist = &h->ht[inet_sk(sk)->inet_num & (RAW_HTABLE_SIZE - 1)]; - write_lock_bh(&h->lock); - sk_add_node(sk, head); + spin_lock(&h->lock); + hlist_nulls_add_head_rcu(&sk->sk_nulls_node, hlist); + sock_set_flag(sk, SOCK_RCU_FREE); sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1); - write_unlock_bh(&h->lock); + spin_unlock(&h->lock); return 0; } @@ -110,10 +109,10 @@ void raw_unhash_sk(struct sock *sk) { struct raw_hashinfo *h = sk->sk_prot->h.raw_hash; - write_lock_bh(&h->lock); - if (sk_del_node_init(sk)) + spin_lock(&h->lock); + if (__sk_nulls_del_node_init_rcu(sk)) sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1); - write_unlock_bh(&h->lock); + spin_unlock(&h->lock); } EXPORT_SYMBOL_GPL(raw_unhash_sk); @@ -121,7 +120,9 @@ struct sock *__raw_v4_lookup(struct net *net, struct sock *sk, unsigned short num, __be32 raddr, __be32 laddr, int dif, int sdif) { - sk_for_each_from(sk) { + struct hlist_nulls_node *hnode; + + sk_nulls_for_each_from(sk, hnode) { struct inet_sock *inet = inet_sk(sk); if (net_eq(sock_net(sk), net) && inet->inet_num == num && @@ -171,17 +172,14 @@ static int raw_v4_input(struct sk_buff *skb, const struct iphdr *iph, int hash) int sdif = inet_sdif(skb); int dif = inet_iif(skb); struct sock *sk; - struct hlist_head *head; + struct hlist_nulls_head *hlist; int delivered = 0; - struct net *net; + struct net *net = dev_net(skb->dev); - read_lock(&raw_v4_hashinfo.lock); - head = &raw_v4_hashinfo.ht[hash]; - if (hlist_empty(head)) - goto out; + hlist = &raw_v4_hashinfo.ht[hash]; + rcu_read_lock(); - net = dev_net(skb->dev); - sk = __raw_v4_lookup(net, __sk_head(head), iph->protocol, + sk = __raw_v4_lookup(net, __sk_nulls_head(hlist), iph->protocol, iph->saddr, iph->daddr, dif, sdif); while (sk) { @@ -195,12 +193,12 @@ static int raw_v4_input(struct sk_buff *skb, const struct iphdr *iph, int hash) if (clone) raw_rcv(sk, clone); } - sk = __raw_v4_lookup(net, sk_next(sk), iph->protocol, + sk = __raw_v4_lookup(net, sk_nulls_next(sk), iph->protocol, iph->saddr, iph->daddr, dif, sdif); } -out: - read_unlock(&raw_v4_hashinfo.lock); + + rcu_read_unlock(); return delivered; } @@ -210,7 +208,7 @@ int raw_local_deliver(struct sk_buff *skb, int protocol) struct sock *raw_sk; hash = protocol & (RAW_HTABLE_SIZE - 1); - raw_sk = sk_head(&raw_v4_hashinfo.ht[hash]); + raw_sk = sk_nulls_head(&raw_v4_hashinfo.ht[hash]); /* If there maybe a raw socket we must check - if not we * don't care less @@ -292,8 +290,8 @@ void raw_icmp_error(struct sk_buff *skb, int protocol, u32 info) hash = protocol & (RAW_HTABLE_SIZE - 1); - read_lock(&raw_v4_hashinfo.lock); - raw_sk = sk_head(&raw_v4_hashinfo.ht[hash]); + rcu_read_lock(); + raw_sk = sk_nulls_head(&raw_v4_hashinfo.ht[hash]); if (raw_sk) { int dif = skb->dev->ifindex; int sdif = inet_sdif(skb); @@ -305,11 +303,11 @@ void raw_icmp_error(struct sk_buff *skb, int protocol, u32 info) iph->daddr, iph->saddr, dif, sdif)) != NULL) { raw_err(raw_sk, skb, info); - raw_sk = sk_next(raw_sk); + raw_sk = sk_nulls_next(raw_sk); iph = (const struct iphdr *)skb->data; } } - read_unlock(&raw_v4_hashinfo.lock); + rcu_read_unlock(); } static int raw_rcv_skb(struct sock *sk, struct sk_buff *skb) @@ -993,10 +991,13 @@ static struct sock *raw_get_first(struct seq_file *seq) struct sock *sk; struct raw_hashinfo *h = PDE_DATA(file_inode(seq->file)); struct raw_iter_state *state = raw_seq_private(seq); + struct hlist_nulls_head *hlist; + struct hlist_nulls_node *hnode; for (state->bucket = 0; state->bucket < RAW_HTABLE_SIZE; ++state->bucket) { - sk_for_each(sk, &h->ht[state->bucket]) + hlist = &h->ht[state->bucket]; + hlist_nulls_for_each_entry(sk, hnode, hlist, sk_nulls_node) if (sock_net(sk) == seq_file_net(seq)) goto found; } @@ -1011,13 +1012,13 @@ static struct sock *raw_get_next(struct seq_file *seq, struct sock *sk) struct raw_iter_state *state = raw_seq_private(seq); do { - sk = sk_next(sk); + sk = sk_nulls_next(sk); try_again: ; } while (sk && sock_net(sk) != seq_file_net(seq)); if (!sk && ++state->bucket < RAW_HTABLE_SIZE) { - sk = sk_head(&h->ht[state->bucket]); + sk = sk_nulls_head(&h->ht[state->bucket]); goto try_again; } return sk; @@ -1035,9 +1036,7 @@ static struct sock *raw_get_idx(struct seq_file *seq, loff_t pos) void *raw_seq_start(struct seq_file *seq, loff_t *pos) { - struct raw_hashinfo *h = PDE_DATA(file_inode(seq->file)); - - read_lock(&h->lock); + rcu_read_lock(); return *pos ? raw_get_idx(seq, *pos - 1) : SEQ_START_TOKEN; } EXPORT_SYMBOL_GPL(raw_seq_start); @@ -1057,9 +1056,7 @@ EXPORT_SYMBOL_GPL(raw_seq_next); void raw_seq_stop(struct seq_file *seq, void *v) { - struct raw_hashinfo *h = PDE_DATA(file_inode(seq->file)); - - read_unlock(&h->lock); + rcu_read_unlock(); } EXPORT_SYMBOL_GPL(raw_seq_stop); diff --git a/net/ipv4/raw_diag.c b/net/ipv4/raw_diag.c index 1b5b8af27..fd7a0fcb8 100644 --- a/net/ipv4/raw_diag.c +++ b/net/ipv4/raw_diag.c @@ -58,15 +58,18 @@ static struct sock *raw_lookup(struct net *net, struct sock *from, static struct sock *raw_sock_get(struct net *net, const struct inet_diag_req_v2 *r) { struct raw_hashinfo *hashinfo = raw_get_hashinfo(r); + struct hlist_nulls_head *hlist; + struct hlist_nulls_node *hnode; struct sock *sk = NULL, *s; int slot; if (IS_ERR(hashinfo)) return ERR_CAST(hashinfo); - read_lock(&hashinfo->lock); + rcu_read_lock(); for (slot = 0; slot < RAW_HTABLE_SIZE; slot++) { - sk_for_each(s, &hashinfo->ht[slot]) { + hlist = &hashinfo->ht[slot]; + hlist_nulls_for_each_entry(sk, hnode, hlist, sk_nulls_node) { sk = raw_lookup(net, s, r); if (sk) { /* @@ -76,13 +79,13 @@ static struct sock *raw_sock_get(struct net *net, const struct inet_diag_req_v2 * We can do that because we're keeping * hashinfo->lock here. */ - sock_hold(sk); - goto out_unlock; + if (refcount_inc_not_zero(&sk->sk_refcnt)) + goto out_unlock; } } } out_unlock: - read_unlock(&hashinfo->lock); + rcu_read_unlock(); return sk ? sk : ERR_PTR(-ENOENT); } @@ -145,6 +148,8 @@ static void raw_diag_dump(struct sk_buff *skb, struct netlink_callback *cb, struct raw_hashinfo *hashinfo = raw_get_hashinfo(r); struct net *net = sock_net(skb->sk); struct inet_diag_dump_data *cb_data; + struct hlist_nulls_head *hlist; + struct hlist_nulls_node *hnode; int num, s_num, slot, s_slot; struct sock *sk = NULL; struct nlattr *bc; @@ -157,11 +162,12 @@ static void raw_diag_dump(struct sk_buff *skb, struct netlink_callback *cb, s_slot = cb->args[0]; num = s_num = cb->args[1]; - read_lock(&hashinfo->lock); + rcu_read_lock(); for (slot = s_slot; slot < RAW_HTABLE_SIZE; s_num = 0, slot++) { num = 0; - sk_for_each(sk, &hashinfo->ht[slot]) { + hlist = &hashinfo->ht[slot]; + hlist_nulls_for_each_entry(sk, hnode, hlist, sk_nulls_node) { struct inet_sock *inet = inet_sk(sk); if (!net_eq(sock_net(sk), net)) @@ -184,7 +190,7 @@ next: } out_unlock: - read_unlock(&hashinfo->lock); + rcu_read_unlock(); cb->args[0] = slot; cb->args[1] = num; diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c index 288c7aa9b..650e49104 100644 --- a/net/ipv6/af_inet6.c +++ b/net/ipv6/af_inet6.c @@ -59,6 +59,7 @@ #endif #include #include +#include #include #include @@ -992,6 +993,8 @@ static int __init inet6_init(void) goto out; } + raw_hashinfo_init(&raw_v6_hashinfo); + err = proto_register(&tcpv6_prot, 1); if (err) goto out; diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c index 828dd9584..a5df873fb 100644 --- a/net/ipv6/raw.c +++ b/net/ipv6/raw.c @@ -61,18 +61,17 @@ #define ICMPV6_HDRLEN 4 /* ICMPv6 header, RFC 4443 Section 2.1 */ -struct raw_hashinfo raw_v6_hashinfo = { - .lock = __RW_LOCK_UNLOCKED(raw_v6_hashinfo.lock), -}; +struct raw_hashinfo raw_v6_hashinfo; EXPORT_SYMBOL_GPL(raw_v6_hashinfo); struct sock *__raw_v6_lookup(struct net *net, struct sock *sk, unsigned short num, const struct in6_addr *loc_addr, const struct in6_addr *rmt_addr, int dif, int sdif) { + struct hlist_nulls_node *hnode; bool is_multicast = ipv6_addr_is_multicast(loc_addr); - sk_for_each_from(sk) + sk_nulls_for_each_from(sk, hnode) if (inet_sk(sk)->inet_num == num) { if (!net_eq(sock_net(sk), net)) @@ -168,8 +167,8 @@ static bool ipv6_raw_deliver(struct sk_buff *skb, int nexthdr) hash = nexthdr & (RAW_HTABLE_SIZE - 1); - read_lock(&raw_v6_hashinfo.lock); - sk = sk_head(&raw_v6_hashinfo.ht[hash]); + rcu_read_lock(); + sk = sk_nulls_head(&raw_v6_hashinfo.ht[hash]); if (!sk) goto out; @@ -219,11 +218,11 @@ static bool ipv6_raw_deliver(struct sk_buff *skb, int nexthdr) rawv6_rcv(sk, clone); } } - sk = __raw_v6_lookup(net, sk_next(sk), nexthdr, daddr, saddr, + sk = __raw_v6_lookup(net, sk_nulls_next(sk), nexthdr, daddr, saddr, inet6_iif(skb), inet6_sdif(skb)); } out: - read_unlock(&raw_v6_hashinfo.lock); + rcu_read_unlock(); return delivered; } @@ -231,7 +230,7 @@ bool raw6_local_deliver(struct sk_buff *skb, int nexthdr) { struct sock *raw_sk; - raw_sk = sk_head(&raw_v6_hashinfo.ht[nexthdr & (RAW_HTABLE_SIZE - 1)]); + raw_sk = sk_nulls_head(&raw_v6_hashinfo.ht[nexthdr & (RAW_HTABLE_SIZE - 1)]); if (raw_sk && !ipv6_raw_deliver(skb, nexthdr)) raw_sk = NULL; @@ -368,8 +367,8 @@ void raw6_icmp_error(struct sk_buff *skb, int nexthdr, hash = nexthdr & (RAW_HTABLE_SIZE - 1); - read_lock(&raw_v6_hashinfo.lock); - sk = sk_head(&raw_v6_hashinfo.ht[hash]); + rcu_read_lock(); + sk = sk_nulls_head(&raw_v6_hashinfo.ht[hash]); if (sk) { /* Note: ipv6_hdr(skb) != skb->data */ const struct ipv6hdr *ip6h = (const struct ipv6hdr *)skb->data; @@ -381,10 +380,10 @@ void raw6_icmp_error(struct sk_buff *skb, int nexthdr, inet6_iif(skb), inet6_iif(skb)))) { rawv6_err(sk, skb, NULL, type, code, inner_offset, info); - sk = sk_next(sk); + sk = sk_nulls_next(sk); } } - read_unlock(&raw_v6_hashinfo.lock); + rcu_read_unlock(); } static inline int rawv6_rcv_skb(struct sock *sk, struct sk_buff *skb)