public-mirrors/linux
1
0
Fork 0
mirror of git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2025-08-05 16:54:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

xfrm: destroy xfrm_state synchronously on net exit path

Browse source 
xfrm_state_put() moves struct xfrm_state to the GC list
and schedules the GC work to clean it up. On net exit call
path, xfrm_state_flush() is called to clean up and
xfrm_flush_gc() is called to wait for the GC work to complete
before exit.

However, this doesn't work because one of the ->destructor(),
ipcomp_destroy(), schedules the same GC work again inside
the GC work. It is hard to wait for such a nested async
callback. This is also why syzbot still reports the following
warning:

 WARNING: CPU: 1 PID: 33 at net/ipv6/xfrm6_tunnel.c:351 xfrm6_tunnel_net_exit+0x2cb/0x500 net/ipv6/xfrm6_tunnel.c:351
 ...
  ops_exit_list.isra.0+0xb0/0x160 net/core/net_namespace.c:153
  cleanup_net+0x51d/0xb10 net/core/net_namespace.c:551
  process_one_work+0xd0c/0x1ce0 kernel/workqueue.c:2153
  worker_thread+0x143/0x14a0 kernel/workqueue.c:2296
  kthread+0x357/0x430 kernel/kthread.c:246
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352

In fact, it is perfectly fine to bypass GC and destroy xfrm_state
synchronously on net exit call path, because it is in process context
and doesn't need a work struct to do any blocking work.

This patch introduces xfrm_state_put_sync() which simply bypasses
GC, and lets its callers to decide whether to use this synchronous
version. On net exit path, xfrm_state_fini() and
xfrm6_tunnel_net_exit() use it. And, as ipcomp_destroy() itself is
blocking, it can use xfrm_state_put_sync() directly too.

Also rename xfrm_state_gc_destroy() to ___xfrm_state_destroy() to
reflect this change.

Fixes: b48c05ab5d ("xfrm: Fix warning in xfrm6_tunnel_net_exit.")
Reported-and-tested-by: syzbot+e9aebef558e3ed673934@syzkaller.appspotmail.com
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
This commit is contained in:
Cong Wang 2019-01-31 13:05:49 -08:00 • committed by Steffen Klassert
parent 09db512411
commit f75a2804da
5 changed files with 31 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
include/net/xfrm.h
View file

@ -853,7 +853,7 @@ static inline void xfrm_pols_put(struct xfrm_policy **pols, int npols)
xfrm_pol_put(pols[i]); xfrm_pol_put(pols[i]);
} }
void __xfrm_state_destroy(struct xfrm_state *); void __xfrm_state_destroy(struct xfrm_state *, bool);
static inline void __xfrm_state_put(struct xfrm_state *x) static inline void __xfrm_state_put(struct xfrm_state *x)
{ {
@ -863,7 +863,13 @@ static inline void __xfrm_state_put(struct xfrm_state *x)
static inline void xfrm_state_put(struct xfrm_state *x) static inline void xfrm_state_put(struct xfrm_state *x)
{ {
if (refcount_dec_and_test(&x->refcnt)) if (refcount_dec_and_test(&x->refcnt))
__xfrm_state_destroy(x); __xfrm_state_destroy(x, false);
}
static inline void xfrm_state_put_sync(struct xfrm_state *x)
{
if (refcount_dec_and_test(&x->refcnt))
__xfrm_state_destroy(x, true);
} }
static inline void xfrm_state_hold(struct xfrm_state *x) static inline void xfrm_state_hold(struct xfrm_state *x)
@ -1590,7 +1596,7 @@ struct xfrmk_spdinfo {
struct xfrm_state *xfrm_find_acq_byseq(struct net *net, u32 mark, u32 seq); struct xfrm_state *xfrm_find_acq_byseq(struct net *net, u32 mark, u32 seq);
int xfrm_state_delete(struct xfrm_state *x); int xfrm_state_delete(struct xfrm_state *x);
int xfrm_state_flush(struct net *net, u8 proto, bool task_valid); int xfrm_state_flush(struct net *net, u8 proto, bool task_valid, bool sync);
int xfrm_dev_state_flush(struct net *net, struct net_device *dev, bool task_valid); int xfrm_dev_state_flush(struct net *net, struct net_device *dev, bool task_valid);
void xfrm_sad_getinfo(struct net *net, struct xfrmk_sadinfo *si); void xfrm_sad_getinfo(struct net *net, struct xfrmk_sadinfo *si);
void xfrm_spd_getinfo(struct net *net, struct xfrmk_spdinfo *si); void xfrm_spd_getinfo(struct net *net, struct xfrmk_spdinfo *si);

2
net/ipv6/xfrm6_tunnel.c
View file

@ -344,8 +344,8 @@ static void __net_exit xfrm6_tunnel_net_exit(struct net *net)
struct xfrm6_tunnel_net *xfrm6_tn = xfrm6_tunnel_pernet(net); struct xfrm6_tunnel_net *xfrm6_tn = xfrm6_tunnel_pernet(net);
unsigned int i; unsigned int i;
xfrm_state_flush(net, IPSEC_PROTO_ANY, false);
xfrm_flush_gc(); xfrm_flush_gc();
xfrm_state_flush(net, IPSEC_PROTO_ANY, false, true);
for (i = 0; i < XFRM6_TUNNEL_SPI_BYADDR_HSIZE; i++) for (i = 0; i < XFRM6_TUNNEL_SPI_BYADDR_HSIZE; i++)
WARN_ON_ONCE(!hlist_empty(&xfrm6_tn->spi_byaddr[i])); WARN_ON_ONCE(!hlist_empty(&xfrm6_tn->spi_byaddr[i]));

2
net/key/af_key.c
View file

@ -1783,7 +1783,7 @@ static int pfkey_flush(struct sock *sk, struct sk_buff *skb, const struct sadb_m
if (proto == 0) if (proto == 0)
return -EINVAL; return -EINVAL;
err = xfrm_state_flush(net, proto, true); err = xfrm_state_flush(net, proto, true, false);
err2 = unicast_flush_resp(sk, hdr); err2 = unicast_flush_resp(sk, hdr);
if (err || err2) { if (err || err2) {
if (err == -ESRCH) /* empty table - go quietly */ if (err == -ESRCH) /* empty table - go quietly */

30
net/xfrm/xfrm_state.c
View file

@ -432,7 +432,7 @@ void xfrm_state_free(struct xfrm_state *x)
} }
EXPORT_SYMBOL(xfrm_state_free); EXPORT_SYMBOL(xfrm_state_free);
static void xfrm_state_gc_destroy(struct xfrm_state *x) static void ___xfrm_state_destroy(struct xfrm_state *x)
{ {
tasklet_hrtimer_cancel(&x->mtimer); tasklet_hrtimer_cancel(&x->mtimer);
del_timer_sync(&x->rtimer); del_timer_sync(&x->rtimer);
@ -474,7 +474,7 @@ static void xfrm_state_gc_task(struct work_struct *work)
synchronize_rcu(); synchronize_rcu();
hlist_for_each_entry_safe(x, tmp, &gc_list, gclist) hlist_for_each_entry_safe(x, tmp, &gc_list, gclist)
xfrm_state_gc_destroy(x); ___xfrm_state_destroy(x);
} }
static enum hrtimer_restart xfrm_timer_handler(struct hrtimer *me) static enum hrtimer_restart xfrm_timer_handler(struct hrtimer *me)
@ -598,14 +598,19 @@ struct xfrm_state *xfrm_state_alloc(struct net *net)
} }
EXPORT_SYMBOL(xfrm_state_alloc); EXPORT_SYMBOL(xfrm_state_alloc);
void __xfrm_state_destroy(struct xfrm_state *x) void __xfrm_state_destroy(struct xfrm_state *x, bool sync)
{ {
WARN_ON(x->km.state != XFRM_STATE_DEAD); WARN_ON(x->km.state != XFRM_STATE_DEAD);
spin_lock_bh(&xfrm_state_gc_lock); if (sync) {
hlist_add_head(&x->gclist, &xfrm_state_gc_list); synchronize_rcu();
spin_unlock_bh(&xfrm_state_gc_lock); ___xfrm_state_destroy(x);
schedule_work(&xfrm_state_gc_work); } else {
spin_lock_bh(&xfrm_state_gc_lock);
hlist_add_head(&x->gclist, &xfrm_state_gc_list);
spin_unlock_bh(&xfrm_state_gc_lock);
schedule_work(&xfrm_state_gc_work);
}
} }
EXPORT_SYMBOL(__xfrm_state_destroy); EXPORT_SYMBOL(__xfrm_state_destroy);
@ -708,7 +713,7 @@ xfrm_dev_state_flush_secctx_check(struct net *net, struct net_device *dev, bool
} }
#endif #endif
int xfrm_state_flush(struct net *net, u8 proto, bool task_valid) int xfrm_state_flush(struct net *net, u8 proto, bool task_valid, bool sync)
{ {
int i, err = 0, cnt = 0; int i, err = 0, cnt = 0;
@ -730,7 +735,10 @@ restart:
err = xfrm_state_delete(x); err = xfrm_state_delete(x);
xfrm_audit_state_delete(x, err ? 0 : 1, xfrm_audit_state_delete(x, err ? 0 : 1,
task_valid); task_valid);
xfrm_state_put(x); if (sync)
xfrm_state_put_sync(x);
else
xfrm_state_put(x);
if (!err) if (!err)
cnt++; cnt++;
@ -2215,7 +2223,7 @@ void xfrm_state_delete_tunnel(struct xfrm_state *x)
if (atomic_read(&t->tunnel_users) == 2) if (atomic_read(&t->tunnel_users) == 2)
xfrm_state_delete(t); xfrm_state_delete(t);
atomic_dec(&t->tunnel_users); atomic_dec(&t->tunnel_users);
xfrm_state_put(t); xfrm_state_put_sync(t);
x->tunnel = NULL; x->tunnel = NULL;
} }
} }
@ -2375,8 +2383,8 @@ void xfrm_state_fini(struct net *net)
unsigned int sz; unsigned int sz;
flush_work(&net->xfrm.state_hash_work); flush_work(&net->xfrm.state_hash_work);
xfrm_state_flush(net, IPSEC_PROTO_ANY, false);
flush_work(&xfrm_state_gc_work); flush_work(&xfrm_state_gc_work);
xfrm_state_flush(net, IPSEC_PROTO_ANY, false, true);
WARN_ON(!list_empty(&net->xfrm.state_all)); WARN_ON(!list_empty(&net->xfrm.state_all));

2
net/xfrm/xfrm_user.c
View file

@ -1932,7 +1932,7 @@ static int xfrm_flush_sa(struct sk_buff *skb, struct nlmsghdr *nlh,
struct xfrm_usersa_flush *p = nlmsg_data(nlh); struct xfrm_usersa_flush *p = nlmsg_data(nlh);
int err; int err;
err = xfrm_state_flush(net, p->proto, true); err = xfrm_state_flush(net, p->proto, true, false);
if (err) { if (err) {
if (err == -ESRCH) /* empty table */ if (err == -ESRCH) /* empty table */
return 0; return 0;