xfrm: delete x->tunnel as we delete x

The ipcomp fallback tunnels currently get deleted (from the various
lists and hashtables) as the last user state that needed that fallback
is destroyed (not deleted). If a reference to that user state still
exists, the fallback state will remain on the hashtables/lists,
triggering the WARN in xfrm_state_fini. Because of those remaining
references, the fix in commit f75a2804da39 ("xfrm: destroy xfrm_state
synchronously on net exit path") is not complete.

We recently fixed one such situation in TCP due to defered freeing of
skbs (commit 9b6412e6979f ("tcp: drop secpath at the same time as we
currently drop dst")). This can also happen due to IP reassembly: skbs
with a secpath remain on the reassembly queue until netns
destruction. If we can't guarantee that the queues are flushed by the
time xfrm_state_fini runs, there may still be references to a (user)
xfrm_state, preventing the timely deletion of the corresponding
fallback state.

Instead of chasing each instance of skbs holding a secpath one by one,
this patch fixes the issue directly within xfrm, by deleting the
fallback state as soon as the last user state depending on it has been
deleted. Destruction will still happen when the final reference is
dropped.

A separate lockdep class for the fallback state is required since
we're going to lock x->tunnel while x is locked.

Fixes: 9d4139c76905 ("netns xfrm: per-netns xfrm_state_all list")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

authored by

Sabrina Dubroca and committed by

Steffen Klassert 11 months ago b441cf3f a90b2a1a

+13 -14

6 changed files

expand all

include

net

xfrm.h

net

ipv4

ipcomp.c

ipv6

ipcomp6.c

xfrm6_tunnel.c

xfrm

xfrm_ipcomp.c

xfrm_state.c

-1

include/net/xfrm.h

··· 441 441 int xfrm_input_unregister_afinfo(const struct xfrm_input_afinfo *afinfo); 442 442 443 443 void xfrm_flush_gc(void); 444 - void xfrm_state_delete_tunnel(struct xfrm_state *x); 445 444 446 445 struct xfrm_type { 447 446 struct module *owner;

net/ipv4/ipcomp.c

··· 54 54 } 55 55 56 56 /* We always hold one tunnel user reference to indicate a tunnel */ 57 + static struct lock_class_key xfrm_state_lock_key; 57 58 static struct xfrm_state *ipcomp_tunnel_create(struct xfrm_state *x) 58 59 { 59 60 struct net *net = xs_net(x); ··· 63 62 t = xfrm_state_alloc(net); 64 63 if (!t) 65 64 goto out; 65 + lockdep_set_class(&t->lock, &xfrm_state_lock_key); 66 66 67 67 t->id.proto = IPPROTO_IPIP; 68 68 t->id.spi = x->props.saddr.a4;

net/ipv6/ipcomp6.c

··· 71 71 return 0; 72 72 } 73 73 74 + static struct lock_class_key xfrm_state_lock_key; 74 75 static struct xfrm_state *ipcomp6_tunnel_create(struct xfrm_state *x) 75 76 { 76 77 struct net *net = xs_net(x); ··· 80 79 t = xfrm_state_alloc(net); 81 80 if (!t) 82 81 goto out; 82 + lockdep_set_class(&t->lock, &xfrm_state_lock_key); 83 83 84 84 t->id.proto = IPPROTO_IPV6; 85 85 t->id.spi = xfrm6_tunnel_alloc_spi(net, (xfrm_address_t *)&x->props.saddr);

+1 -1

net/ipv6/xfrm6_tunnel.c

··· 334 334 struct xfrm6_tunnel_net *xfrm6_tn = xfrm6_tunnel_pernet(net); 335 335 unsigned int i; 336 336 337 - xfrm_flush_gc(); 338 337 xfrm_state_flush(net, 0, false, true); 338 + xfrm_flush_gc(); 339 339 340 340 for (i = 0; i < XFRM6_TUNNEL_SPI_BYADDR_HSIZE; i++) 341 341 WARN_ON_ONCE(!hlist_empty(&xfrm6_tn->spi_byaddr[i]));

-1

net/xfrm/xfrm_ipcomp.c

··· 313 313 struct ipcomp_data *ipcd = x->data; 314 314 if (!ipcd) 315 315 return; 316 - xfrm_state_delete_tunnel(x); 317 316 ipcomp_free_data(ipcd); 318 317 kfree(ipcd); 319 318 }

+8 -11

net/xfrm/xfrm_state.c

··· 811 811 } 812 812 EXPORT_SYMBOL(__xfrm_state_destroy); 813 813 814 + static void xfrm_state_delete_tunnel(struct xfrm_state *x); 814 815 int __xfrm_state_delete(struct xfrm_state *x) 815 816 { 816 817 struct net *net = xs_net(x); ··· 838 837 spin_unlock(&net->xfrm.xfrm_state_lock); 839 838 840 839 xfrm_dev_state_delete(x); 840 + 841 + xfrm_state_delete_tunnel(x); 841 842 842 843 /* All xfrm_state objects are created by xfrm_state_alloc. 843 844 * The xfrm_state_alloc call gives a reference, and that ··· 944 941 err = xfrm_state_delete(x); 945 942 xfrm_audit_state_delete(x, err ? 0 : 1, 946 943 task_valid); 947 - if (sync) 948 - xfrm_state_put_sync(x); 949 - else 950 - xfrm_state_put(x); 944 + xfrm_state_put(x); 951 945 if (!err) 952 946 cnt++; 953 947 ··· 3068 3068 } 3069 3069 EXPORT_SYMBOL(xfrm_flush_gc); 3070 3070 3071 - /* Temporarily located here until net/xfrm/xfrm_tunnel.c is created */ 3072 - void xfrm_state_delete_tunnel(struct xfrm_state *x) 3071 + static void xfrm_state_delete_tunnel(struct xfrm_state *x) 3073 3072 { 3074 3073 if (x->tunnel) { 3075 3074 struct xfrm_state *t = x->tunnel; 3076 3075 3077 - if (atomic_read(&t->tunnel_users) == 2) 3076 + if (atomic_dec_return(&t->tunnel_users) == 1) 3078 3077 xfrm_state_delete(t); 3079 - atomic_dec(&t->tunnel_users); 3080 - xfrm_state_put_sync(t); 3078 + xfrm_state_put(t); 3081 3079 x->tunnel = NULL; 3082 3080 } 3083 3081 } 3084 - EXPORT_SYMBOL(xfrm_state_delete_tunnel); 3085 3082 3086 3083 u32 xfrm_state_mtu(struct xfrm_state *x, int mtu) 3087 3084 { ··· 3283 3286 unsigned int sz; 3284 3287 3285 3288 flush_work(&net->xfrm.state_hash_work); 3286 - flush_work(&xfrm_state_gc_work); 3287 3289 xfrm_state_flush(net, 0, false, true); 3290 + flush_work(&xfrm_state_gc_work); 3288 3291 3289 3292 WARN_ON(!list_empty(&net->xfrm.state_all)); 3290 3293

Configure Feed

Configure Feed