Skip to content

Commit 5c86c89

Browse files
Zihan Xigregkh
authored andcommitted
xfrm: avoid lock inversion in nat keepalive work
commit 763fe70 upstream. 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 the keepalive walk into two phases. First, collect the candidate states while the walk holds xfrm_state_lock and take a reference on each state. Then, after the walk completes, process each collected state and acquire x->lock without nesting it under xfrm_state_lock. Fixes: f531d13 ("xfrm: support sending NAT keepalives in ESP in UDP states") Cc: stable@vger.kernel.org Reported-by: Vega <vega@nebusec.ai> Assisted-by: Codex:gpt-5.4 Signed-off-by: Zihan Xi <xizh2024@lzu.edu.cn> Signed-off-by: Ren Wei <enjou1224z@gmail.com> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 328e40a commit 5c86c89

1 file changed

Lines changed: 48 additions & 9 deletions

File tree

net/xfrm/xfrm_nat_keepalive.c

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -156,24 +156,51 @@ static void nat_keepalive_send(struct nat_keepalive *ka)
156156
}
157157

158158
struct nat_keepalive_work_ctx {
159+
struct list_head states;
159160
time64_t next_run;
160161
time64_t now;
161162
};
162163

163-
static int nat_keepalive_work_single(struct xfrm_state *x, int count, void *ptr)
164+
struct nat_keepalive_state {
165+
struct list_head list;
166+
struct xfrm_state *x;
167+
};
168+
169+
static int nat_keepalive_work_collect(struct xfrm_state *x, int count, void *ptr)
164170
{
165171
struct nat_keepalive_work_ctx *ctx = ptr;
172+
struct nat_keepalive_state *state;
173+
174+
if (!READ_ONCE(x->nat_keepalive_interval))
175+
return 0;
176+
177+
state = kmalloc_obj(*state, GFP_ATOMIC);
178+
if (!state)
179+
return -ENOMEM;
180+
181+
xfrm_state_hold(x);
182+
state->x = x;
183+
list_add_tail(&state->list, &ctx->states);
184+
return 0;
185+
}
186+
187+
static void nat_keepalive_work_single(struct xfrm_state *x,
188+
struct nat_keepalive_work_ctx *ctx)
189+
{
166190
bool send_keepalive = false;
167191
struct nat_keepalive ka;
168-
time64_t next_run;
192+
time64_t next_run = 0;
169193
u32 interval;
170194
int delta;
171195

196+
spin_lock_bh(&x->lock);
197+
198+
if (x->km.state == XFRM_STATE_DEAD)
199+
goto out;
200+
172201
interval = x->nat_keepalive_interval;
173202
if (!interval)
174-
return 0;
175-
176-
spin_lock(&x->lock);
203+
goto out;
177204

178205
delta = (int)(ctx->now - x->lastused);
179206
if (delta < interval) {
@@ -187,29 +214,41 @@ static int nat_keepalive_work_single(struct xfrm_state *x, int count, void *ptr)
187214
send_keepalive = true;
188215
}
189216

190-
spin_unlock(&x->lock);
217+
out:
218+
spin_unlock_bh(&x->lock);
191219

192220
if (send_keepalive)
193221
nat_keepalive_send(&ka);
194222

195-
if (!ctx->next_run || next_run < ctx->next_run)
223+
if (next_run && (!ctx->next_run || next_run < ctx->next_run))
196224
ctx->next_run = next_run;
197-
return 0;
198225
}
199226

200227
static void nat_keepalive_work(struct work_struct *work)
201228
{
229+
struct nat_keepalive_state *state, *tmp;
202230
struct nat_keepalive_work_ctx ctx;
203231
struct xfrm_state_walk walk;
204232
struct net *net;
233+
int err;
205234

235+
INIT_LIST_HEAD(&ctx.states);
206236
ctx.next_run = 0;
207237
ctx.now = ktime_get_real_seconds();
208238

209239
net = container_of(work, struct net, xfrm.nat_keepalive_work.work);
210240
xfrm_state_walk_init(&walk, IPPROTO_ESP, NULL);
211-
xfrm_state_walk(net, &walk, nat_keepalive_work_single, &ctx);
241+
err = xfrm_state_walk(net, &walk, nat_keepalive_work_collect, &ctx);
212242
xfrm_state_walk_done(&walk, net);
243+
list_for_each_entry_safe(state, tmp, &ctx.states, list) {
244+
nat_keepalive_work_single(state->x, &ctx);
245+
xfrm_state_put(state->x);
246+
kfree(state);
247+
}
248+
if (err == -ENOMEM) {
249+
schedule_delayed_work(&net->xfrm.nat_keepalive_work, 0);
250+
return;
251+
}
213252
if (ctx.next_run)
214253
schedule_delayed_work(&net->xfrm.nat_keepalive_work,
215254
(ctx.next_run - ctx.now) * HZ);

0 commit comments

Comments
 (0)