xfrm: destroy xfrm_state synchronously on net exit path
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:

committed by
Steffen Klassert

parent
09db512411
commit
f75a2804da
@@ -853,7 +853,7 @@ static inline void xfrm_pols_put(struct xfrm_policy **pols, int npols)
|
||||
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)
|
||||
{
|
||||
@@ -863,7 +863,13 @@ 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))
|
||||
__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)
|
||||
@@ -1590,7 +1596,7 @@ struct xfrmk_spdinfo {
|
||||
|
||||
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_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);
|
||||
void xfrm_sad_getinfo(struct net *net, struct xfrmk_sadinfo *si);
|
||||
void xfrm_spd_getinfo(struct net *net, struct xfrmk_spdinfo *si);
|
||||
|
Reference in New Issue
Block a user