[PATCH net v2 1/1] xfrm: avoid lock inversion in nat keepalive work
From: Zihan Xi <hidden>
Date: 2026-08-05 04:51:58
Subsystem:
networking [general], networking [ipsec], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Steffen Klassert, Herbert Xu, Linus Torvalds
nat_keepalive_work() walks the state table while xfrm_state_walk()
holds net->xfrm.xfrm_state_lock. Its callback then acquires x->lock,
which conflicts with the delete path taking the same locks in reverse
order via xfrm_state_delete() and __xfrm_state_delete(). This creates
an AB-BA deadlock that is reported by lockdep when a NAT keepalive
worker races with SA deletion.
Fix this by splitting keepalive handling into two phases without
reversing the lock ordering. Walk the state table only to collect a
bounded batch of candidate states while holding xfrm_state_lock and
taking references on them. Then, after the walk drops xfrm_state_lock,
process each referenced state and take x->lock in phase two.
Use a small fixed-size batch and the xfrm_state_walk() cursor to resume
the scan when the batch fills up. This avoids per-state GFP_ATOMIC
allocations, avoids aborting a full round on -ENOMEM, and keeps
nat_keepalive_send() in BH-disabled context by wrapping the phase-two
drain with local_bh_disable()/local_bh_enable().
Fixes: f531d13bdfe3 ("xfrm: support sending NAT keepalives in ESP in UDP states")
Cc: stable@vger.kernel.org
Cc: Eyal Birger <redacted>
Reported-by: Vega <redacted>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <redacted>
---
changes in v2:
- reroll on top of net cf6f8b29befb so the patch applies after the
nat_keepalive_send() default-case change
- replace the unbounded GFP_ATOMIC state list with a bounded batch
- keep phase-two processing in BH-disabled context with
local_bh_disable()/local_bh_enable()
- clarify the validation permission model and reproducer scope in
the cover letter
- refresh the cover letter with cf6f8b29 incremental build and PoC
validation results
- rerun decode_stacktrace.sh on an LF-normalized crash log and
document the decoder truncation in the cover letter
- add source locations for the key lockdep offsets from the same
unfixed vmlinux
- add Eyal Birger to Cc for the v2 reroll
- v1 Link: https://lore.kernel.org/all/cover.1784645321.git.xizh2024@lzu.edu.cn/ (local)
net/xfrm/xfrm_nat_keepalive.c | 51 +++++++++++++++++++++++++++++------
1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/net/xfrm/xfrm_nat_keepalive.c b/net/xfrm/xfrm_nat_keepalive.c
index eb1b6f677..bcc351625 100644
--- a/net/xfrm/xfrm_nat_keepalive.c
+++ b/net/xfrm/xfrm_nat_keepalive.c@@ -155,25 +155,50 @@ static void nat_keepalive_send(struct nat_keepalive *ka) } } +enum { + NAT_KEEPALIVE_BATCH_SIZE = 16, + NAT_KEEPALIVE_BATCH_FULL = 1, +}; + struct nat_keepalive_work_ctx { + struct xfrm_state *batch[NAT_KEEPALIVE_BATCH_SIZE]; + unsigned int nr; time64_t next_run; time64_t now; }; -static int nat_keepalive_work_single(struct xfrm_state *x, int count, void *ptr) +static int nat_keepalive_work_collect(struct xfrm_state *x, int count, void *ptr) { struct nat_keepalive_work_ctx *ctx = ptr; + + if (!READ_ONCE(x->nat_keepalive_interval)) + return 0; + + if (ctx->nr == ARRAY_SIZE(ctx->batch)) + return NAT_KEEPALIVE_BATCH_FULL; + + xfrm_state_hold(x); + ctx->batch[ctx->nr++] = x; + return 0; +} + +static void nat_keepalive_work_single(struct xfrm_state *x, + struct nat_keepalive_work_ctx *ctx) +{ bool send_keepalive = false; struct nat_keepalive ka; - time64_t next_run; + time64_t next_run = 0; u32 interval; int delta; + spin_lock(&x->lock); + + if (x->km.state == XFRM_STATE_DEAD) + goto out; + interval = x->nat_keepalive_interval; if (!interval) - return 0; - - spin_lock(&x->lock); + goto out; delta = (int)(ctx->now - x->lastused); if (delta < interval) {
@@ -187,14 +212,14 @@ static int nat_keepalive_work_single(struct xfrm_state *x, int count, void *ptr) send_keepalive = true; } +out: spin_unlock(&x->lock); if (send_keepalive) nat_keepalive_send(&ka); - if (!ctx->next_run || next_run < ctx->next_run) + if (next_run && (!ctx->next_run || next_run < ctx->next_run)) ctx->next_run = next_run; - return 0; } static void nat_keepalive_work(struct work_struct *work)
@@ -202,13 +227,23 @@ static void nat_keepalive_work(struct work_struct *work) struct nat_keepalive_work_ctx ctx; struct xfrm_state_walk walk; struct net *net; + int err, i; ctx.next_run = 0; ctx.now = ktime_get_real_seconds(); net = container_of(work, struct net, xfrm.nat_keepalive_work.work); xfrm_state_walk_init(&walk, IPPROTO_ESP, NULL); - xfrm_state_walk(net, &walk, nat_keepalive_work_single, &ctx); + do { + ctx.nr = 0; + err = xfrm_state_walk(net, &walk, nat_keepalive_work_collect, &ctx); + for (i = 0; i < ctx.nr; i++) { + local_bh_disable(); + nat_keepalive_work_single(ctx.batch[i], &ctx); + local_bh_enable(); + xfrm_state_put(ctx.batch[i]); + } + } while (err == NAT_KEEPALIVE_BATCH_FULL); xfrm_state_walk_done(&walk, net); if (ctx.next_run) schedule_delayed_work(&net->xfrm.nat_keepalive_work,
--
2.43.0