From b07dd26f07af294ceed9715fd11e312ff8de6138 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Wed, 14 Apr 2021 18:12:51 +0200 Subject: [PATCH 1/4] flow: remove spi key from flowi struct xfrm session decode ipv4 path (but not ipv6) sets this, but there are no consumers. Remove it. Signed-off-by: Florian Westphal Signed-off-by: Steffen Klassert --- include/net/flow.h | 3 --- net/xfrm/xfrm_policy.c | 39 --------------------------------------- 2 files changed, 42 deletions(-) diff --git a/include/net/flow.h b/include/net/flow.h index 39d0cedcddeee..6f5e702400717 100644 --- a/include/net/flow.h +++ b/include/net/flow.h @@ -59,7 +59,6 @@ union flowi_uli { __le16 sport; } dnports; - __be32 spi; __be32 gre_key; struct { @@ -90,7 +89,6 @@ struct flowi4 { #define fl4_dport uli.ports.dport #define fl4_icmp_type uli.icmpt.type #define fl4_icmp_code uli.icmpt.code -#define fl4_ipsec_spi uli.spi #define fl4_mh_type uli.mht.type #define fl4_gre_key uli.gre_key } __attribute__((__aligned__(BITS_PER_LONG/8))); @@ -150,7 +148,6 @@ struct flowi6 { #define fl6_dport uli.ports.dport #define fl6_icmp_type uli.icmpt.type #define fl6_icmp_code uli.icmpt.code -#define fl6_ipsec_spi uli.spi #define fl6_mh_type uli.mht.type #define fl6_gre_key uli.gre_key __u32 mp_hash; diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 156347fd7e2ec..cc6e02eb76c28 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -3326,39 +3326,6 @@ decode_session4(struct sk_buff *skb, struct flowi *fl, bool reverse) fl4->fl4_icmp_code = icmp[1]; } break; - case IPPROTO_ESP: - if (xprth + 4 < skb->data || - pskb_may_pull(skb, xprth + 4 - skb->data)) { - __be32 *ehdr; - - xprth = skb_network_header(skb) + ihl * 4; - ehdr = (__be32 *)xprth; - - fl4->fl4_ipsec_spi = ehdr[0]; - } - break; - case IPPROTO_AH: - if (xprth + 8 < skb->data || - pskb_may_pull(skb, xprth + 8 - skb->data)) { - __be32 *ah_hdr; - - xprth = skb_network_header(skb) + ihl * 4; - ah_hdr = (__be32 *)xprth; - - fl4->fl4_ipsec_spi = ah_hdr[1]; - } - break; - case IPPROTO_COMP: - if (xprth + 4 < skb->data || - pskb_may_pull(skb, xprth + 4 - skb->data)) { - __be16 *ipcomp_hdr; - - xprth = skb_network_header(skb) + ihl * 4; - ipcomp_hdr = (__be16 *)xprth; - - fl4->fl4_ipsec_spi = htonl(ntohs(ipcomp_hdr[1])); - } - break; case IPPROTO_GRE: if (xprth + 12 < skb->data || pskb_may_pull(skb, xprth + 12 - skb->data)) { @@ -3377,7 +3344,6 @@ decode_session4(struct sk_buff *skb, struct flowi *fl, bool reverse) } break; default: - fl4->fl4_ipsec_spi = 0; break; } } @@ -3470,12 +3436,7 @@ decode_session6(struct sk_buff *skb, struct flowi *fl, bool reverse) fl6->flowi6_proto = nexthdr; return; #endif - /* XXX Why are there these headers? */ - case IPPROTO_AH: - case IPPROTO_ESP: - case IPPROTO_COMP: default: - fl6->fl6_ipsec_spi = 0; fl6->flowi6_proto = nexthdr; return; } From 7baf867fef7cc65d666792e9d1b911beffe74ad7 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Wed, 14 Apr 2021 18:12:52 +0200 Subject: [PATCH 2/4] xfrm: remove stray synchronize_rcu from xfrm_init This function is called during boot, from ipv4 stack, there is no need to set the pointer to NULL (static storage duration, so already NULL). No need for the synchronize_rcu either. Remove both. Signed-off-by: Florian Westphal Signed-off-by: Steffen Klassert --- net/xfrm/xfrm_policy.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index cc6e02eb76c28..ce500f847b991 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -4134,9 +4134,6 @@ void __init xfrm_init(void) #ifdef CONFIG_XFRM_ESPINTCP espintcp_init(); #endif - - RCU_INIT_POINTER(xfrm_if_cb, NULL); - synchronize_rcu(); } #ifdef CONFIG_AUDITSYSCALL From 6218fe186109b93a2fa2343e13981e016e9961ab Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Wed, 14 Apr 2021 18:12:53 +0200 Subject: [PATCH 3/4] xfrm: avoid synchronize_rcu during netns destruction Use the new exit_pre hook to NULL the netlink socket. The net namespace core will do a synchronize_rcu() between the exit_pre and exit/exit_batch handlers. Signed-off-by: Florian Westphal Signed-off-by: Steffen Klassert --- net/xfrm/xfrm_user.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c index df8bc8fc724cc..f0aecee4d5392 100644 --- a/net/xfrm/xfrm_user.c +++ b/net/xfrm/xfrm_user.c @@ -3480,18 +3480,22 @@ static int __net_init xfrm_user_net_init(struct net *net) return 0; } +static void __net_exit xfrm_user_net_pre_exit(struct net *net) +{ + RCU_INIT_POINTER(net->xfrm.nlsk, NULL); +} + static void __net_exit xfrm_user_net_exit(struct list_head *net_exit_list) { struct net *net; - list_for_each_entry(net, net_exit_list, exit_list) - RCU_INIT_POINTER(net->xfrm.nlsk, NULL); - synchronize_net(); + list_for_each_entry(net, net_exit_list, exit_list) netlink_kernel_release(net->xfrm.nlsk_stash); } static struct pernet_operations xfrm_user_net_ops = { .init = xfrm_user_net_init, + .pre_exit = xfrm_user_net_pre_exit, .exit_batch = xfrm_user_net_exit, }; From 747b67088f8d34b3ec64d31447a1044be92dd348 Mon Sep 17 00:00:00 2001 From: Sabrina Dubroca Date: Fri, 16 Apr 2021 17:11:46 +0200 Subject: [PATCH 4/4] xfrm: ipcomp: remove unnecessary get_cpu() While testing ipcomp on a realtime kernel, Xiumei reported a "sleeping in atomic" bug, caused by a memory allocation while preemption is disabled (ipcomp_decompress -> alloc_page -> ... get_page_from_freelist). As Sebastian noted [1], this get_cpu() isn't actually needed, since ipcomp_decompress() is called in napi context anyway, so BH is already disabled. This patch replaces get_cpu + per_cpu_ptr with this_cpu_ptr, then simplifies the error returns, since there isn't any common operation left. [1] https://lore.kernel.org/lkml/20190820082810.ixkmi56fp7u7eyn2@linutronix.de/ Cc: Juri Lelli Reported-by: Xiumei Mu Suggested-by: Sebastian Andrzej Siewior Signed-off-by: Sabrina Dubroca Signed-off-by: Steffen Klassert --- net/xfrm/xfrm_ipcomp.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c index 4d422447aadc3..2e8afe078d612 100644 --- a/net/xfrm/xfrm_ipcomp.c +++ b/net/xfrm/xfrm_ipcomp.c @@ -41,19 +41,16 @@ static int ipcomp_decompress(struct xfrm_state *x, struct sk_buff *skb) const int plen = skb->len; int dlen = IPCOMP_SCRATCH_SIZE; const u8 *start = skb->data; - const int cpu = get_cpu(); - u8 *scratch = *per_cpu_ptr(ipcomp_scratches, cpu); - struct crypto_comp *tfm = *per_cpu_ptr(ipcd->tfms, cpu); + u8 *scratch = *this_cpu_ptr(ipcomp_scratches); + struct crypto_comp *tfm = *this_cpu_ptr(ipcd->tfms); int err = crypto_comp_decompress(tfm, start, plen, scratch, &dlen); int len; if (err) - goto out; + return err; - if (dlen < (plen + sizeof(struct ip_comp_hdr))) { - err = -EINVAL; - goto out; - } + if (dlen < (plen + sizeof(struct ip_comp_hdr))) + return -EINVAL; len = dlen - plen; if (len > skb_tailroom(skb)) @@ -68,16 +65,14 @@ static int ipcomp_decompress(struct xfrm_state *x, struct sk_buff *skb) skb_frag_t *frag; struct page *page; - err = -EMSGSIZE; if (WARN_ON(skb_shinfo(skb)->nr_frags >= MAX_SKB_FRAGS)) - goto out; + return -EMSGSIZE; frag = skb_shinfo(skb)->frags + skb_shinfo(skb)->nr_frags; page = alloc_page(GFP_ATOMIC); - err = -ENOMEM; if (!page) - goto out; + return -ENOMEM; __skb_frag_set_page(frag, page); @@ -96,11 +91,7 @@ static int ipcomp_decompress(struct xfrm_state *x, struct sk_buff *skb) skb_shinfo(skb)->nr_frags++; } - err = 0; - -out: - put_cpu(); - return err; + return 0; } int ipcomp_input(struct xfrm_state *x, struct sk_buff *skb)