Skip to content

Commit 71cdc4b

Browse files
Zihan Xigregkh
authored andcommitted
xfrm: bound nat keepalive state collection
commit 4e9442c upstream. The v1 nat keepalive fix allocates a GFP_ATOMIC object for every state while collecting references for phase two. This makes the worker's temporary memory use depend on the number of states and lets -ENOMEM abort the scan. Replace the allocated list with a fixed-size batch. When the batch is full, return a private walk status so xfrm_state_walk() leaves a cursor; drain the references after the walk releases xfrm_state_lock and resume from the cursor. This bounds temporary memory use and avoids the allocation failure path. The v1 fix also moved nat_keepalive_send() out of the walk callback. Keep the phase-two drain BH-disabled, as required by local_lock_nested_bh() used by the keepalive sockets. Fixes: 763fe70 ("xfrm: avoid lock inversion in nat keepalive work") Cc: stable@vger.kernel.org Cc: Eyal Birger <eyal.birger@gmail.com> Reported-by: Vega <vega@nebusec.ai> Assisted-by: Codex:gpt-5.4 Signed-off-by: Zihan Xi <zihanx@nebusec.ai> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 71d42da commit 71cdc4b

1 file changed

Lines changed: 21 additions & 25 deletions

File tree

net/xfrm/xfrm_nat_keepalive.c

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -155,32 +155,30 @@ static void nat_keepalive_send(struct nat_keepalive *ka)
155155
}
156156
}
157157

158+
enum {
159+
NAT_KEEPALIVE_BATCH_SIZE = 16,
160+
NAT_KEEPALIVE_BATCH_FULL = 1,
161+
};
162+
158163
struct nat_keepalive_work_ctx {
159-
struct list_head states;
164+
struct xfrm_state *batch[NAT_KEEPALIVE_BATCH_SIZE];
165+
unsigned int nr;
160166
time64_t next_run;
161167
time64_t now;
162168
};
163169

164-
struct nat_keepalive_state {
165-
struct list_head list;
166-
struct xfrm_state *x;
167-
};
168-
169170
static int nat_keepalive_work_collect(struct xfrm_state *x, int count, void *ptr)
170171
{
171172
struct nat_keepalive_work_ctx *ctx = ptr;
172-
struct nat_keepalive_state *state;
173173

174174
if (!READ_ONCE(x->nat_keepalive_interval))
175175
return 0;
176176

177-
state = kmalloc_obj(*state, GFP_ATOMIC);
178-
if (!state)
179-
return -ENOMEM;
177+
if (ctx->nr == ARRAY_SIZE(ctx->batch))
178+
return NAT_KEEPALIVE_BATCH_FULL;
180179

181180
xfrm_state_hold(x);
182-
state->x = x;
183-
list_add_tail(&state->list, &ctx->states);
181+
ctx->batch[ctx->nr++] = x;
184182
return 0;
185183
}
186184

@@ -226,29 +224,27 @@ static void nat_keepalive_work_single(struct xfrm_state *x,
226224

227225
static void nat_keepalive_work(struct work_struct *work)
228226
{
229-
struct nat_keepalive_state *state, *tmp;
230227
struct nat_keepalive_work_ctx ctx;
231228
struct xfrm_state_walk walk;
232229
struct net *net;
233-
int err;
230+
int err, i;
234231

235-
INIT_LIST_HEAD(&ctx.states);
236232
ctx.next_run = 0;
237233
ctx.now = ktime_get_real_seconds();
238234

239235
net = container_of(work, struct net, xfrm.nat_keepalive_work.work);
240236
xfrm_state_walk_init(&walk, IPPROTO_ESP, NULL);
241-
err = xfrm_state_walk(net, &walk, nat_keepalive_work_collect, &ctx);
237+
do {
238+
ctx.nr = 0;
239+
err = xfrm_state_walk(net, &walk, nat_keepalive_work_collect, &ctx);
240+
local_bh_disable();
241+
for (i = 0; i < ctx.nr; i++) {
242+
nat_keepalive_work_single(ctx.batch[i], &ctx);
243+
xfrm_state_put(ctx.batch[i]);
244+
}
245+
local_bh_enable();
246+
} while (err == NAT_KEEPALIVE_BATCH_FULL);
242247
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-
}
252248
if (ctx.next_run)
253249
schedule_delayed_work(&net->xfrm.nat_keepalive_work,
254250
(ctx.next_run - ctx.now) * HZ);

0 commit comments

Comments
 (0)