[PATCH 0/1] xfrm: avoid lock inversion in nat keepalive work

[Ren Wei <enjou1224z@gmail.com>]

From: Zihan Xi <xizh2024@lzu.edu.cn>

Hi Linux kernel maintainers,

We found and validated a lock inversion issue in
net/xfrm/xfrm_nat_keepalive.c. The bug is reachable when an outbound
ESP-in-UDP state enables NAT keepalives and races with SA deletion.
We've tested the fix, and it does not affect the normal delete path.

This series contains one patch:

  1/1 xfrm: avoid lock inversion in nat keepalive work

We provide bug details, reproducer steps, and a crash log below.

---- details below ----

Bug details:

nat_keepalive_work() walks the state table through xfrm_state_walk()
while xfrm_state_walk() holds net->xfrm.xfrm_state_lock. In the buggy
code, the walk callback nat_keepalive_work_single() then acquires
x->lock. The delete path takes the reverse order:
xfrm_state_delete() acquires x->lock first, and __xfrm_state_delete()
later acquires net->xfrm.xfrm_state_lock. That creates an AB-BA lock
inversion between the keepalive worker and the delete path, and lockdep
reports it as a circular dependency.

The fix keeps xfrm_state_lock out of the per-state processing phase.
The worker first collects matching states under the walk with an extra
reference, then processes them after the walk finishes and only then
takes x->lock. This preserves the original logic while avoiding the
reverse lock nesting.

We first tried to validate this with the original userspace
NETLINK_XFRM reproducer in the bug directory. On this validation kernel,
that reproducer was rejected during strict attribute validation with
"attribute type 34 has an invalid length" before it could reach the
buggy path. To validate the same root cause and the same keepalive
worker versus SA delete ordering in-kernel, we used a temporary local
in-kernel reproducer that creates an outbound ESP-in-UDP state with
NAT keepalive enabled, flushes the keepalive worker, and then deletes
the state. This temporary reproducer was used only for local validation
and is not part of the patch series.

Reproducer:

    # Build kernels with the temporary local in-kernel reproducer linked in
    make -C /var/cache/linux-patch/xfrm-nat-keepalive-src \
        O=/var/cache/linux-patch/bt-parent-uaf-net-main-build -j$(nproc) bzImage

    # Boot the unfixed kernel
    qemu-system-x86_64 -m 2G -cpu max -smp 2 -machine accel=tcg \
        -kernel verify/bzImage-unfixed-kpoc \
        -append 'root=/dev/sda rw console=ttyS0 earlyprintk=serial \
        net.ifnames=0 biosdevname=0 panic_on_warn=1 oops=panic \
        slub_debug=FZPU page_poison=1 init_on_alloc=1 init_on_free=1' \
        -drive file=/tmp/qemu-unfixed-realboot.qcow2,format=qcow2

    # Boot the fixed kernel
    qemu-system-x86_64 -m 2G -cpu max -smp 2 -machine accel=tcg \
        -kernel verify/bzImage-fixed-kpoc \
        -append 'root=/dev/sda rw console=ttyS0 earlyprintk=serial \
        net.ifnames=0 biosdevname=0 panic_on_warn=1 oops=panic \
        slub_debug=FZPU page_poison=1 init_on_alloc=1 init_on_free=1' \
        -drive file=/tmp/qemu-fixed-realboot.qcow2,format=qcow2

We run the validation in a 2 vCPU, 2 GB RAM x86 QEMU environment.

------BEGIN xfrm_nat_keepalive_repro.c------

// SPDX-License-Identifier: GPL-2.0
#include <linux/init.h>
#include <linux/in.h>
#include <linux/ip.h>
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/workqueue.h>
#include <net/net_namespace.h>
#include <net/xfrm.h>

static int __init xfrm_nat_keepalive_repro_init(void)
{
	struct xfrm_state *x;
	struct xfrm_encap_tmpl *encap;
	int err;

	pr_info("xfrm_nat_keepalive_repro: start\n");

	x = xfrm_state_alloc(&init_net);
	if (!x)
		return 0;

	encap = kzalloc(sizeof(*encap), GFP_KERNEL);
	if (!encap) {
		xfrm_state_put(x);
		return 0;
	}

	encap->encap_type = UDP_ENCAP_ESPINUDP;
	encap->encap_sport = htons(4500);
	encap->encap_dport = htons(4500);

	x->id.proto = IPPROTO_ESP;
	x->id.spi = htonl(0x100);
	x->id.daddr.a4 = htonl(INADDR_LOOPBACK);
	x->props.saddr.a4 = htonl(INADDR_LOOPBACK);
	x->props.family = AF_INET;
	x->props.mode = XFRM_MODE_TRANSPORT;
	x->props.reqid = 1;
	x->sel.family = AF_INET;
	x->sel.daddr.a4 = htonl(INADDR_LOOPBACK);
	x->sel.saddr.a4 = htonl(INADDR_LOOPBACK);
	x->sel.prefixlen_d = 32;
	x->sel.prefixlen_s = 32;
	x->encap = encap;
	x->dir = XFRM_SA_DIR_OUT;
	x->nat_keepalive_interval = 1;
	x->lastused = ktime_get_real_seconds();
	x->km.state = XFRM_STATE_VALID;

	xfrm_state_insert(x);
	flush_delayed_work(&init_net.xfrm.nat_keepalive_work);
	err = xfrm_state_delete(x);
	xfrm_flush_gc();
	pr_info("xfrm_nat_keepalive_repro: delete err=%d\n", err);
	return 0;
}

late_initcall_sync(xfrm_nat_keepalive_repro_init);

------END xfrm_nat_keepalive_repro.c--------

----BEGIN crash log----

[  129.325779][    T1] xfrm_nat_keepalive_repro: start
[  129.348170][    T1] WARNING: possible circular locking dependency detected
[  129.348170][    T1] 7.2.0-rc2+ #5 Not tainted
[  129.348170][    T1] ------------------------------------------------------
[  129.348170][    T1] swapper/0/1 is trying to acquire lock:
[  129.348170][    T1] ffffffff989f9fd8 (&net->xfrm.xfrm_state_lock){+...}-{3:3}, at: __xfrm_state_delete+0xa4/0x9d0
[  129.460853][    T1] but task is already holding lock:
[  129.460853][    T1] ff1100001bbfc0c8 (&x->lock){+...}-{3:3}, at: xfrm_state_delete+0x1b/0x40
[  129.460853][    T1] -> #1 (&x->lock){+...}-{3:3}:
[  129.460853][    T1]        _raw_spin_lock+0x2d/0x40
[  129.460853][    T1]        nat_keepalive_work_single+0x15c/0x1c40
[  129.460853][    T1]        xfrm_state_walk+0x4ed/0xb70
[  129.460853][    T1]        nat_keepalive_work+0xe8/0x1b0
[  129.460853][    T1] -> #0 (&net->xfrm.xfrm_state_lock){+...}-{3:3}:
[  129.460853][    T1]        __xfrm_state_delete+0xa4/0x9d0
[  129.460853][    T1]        xfrm_state_delete+0x23/0x40
[  129.460853][    T1]        xfrm_nat_keepalive_repro_init+0x4ba/0x640
[  129.460853][    T1]  *** DEADLOCK ***
[  130.799594][    T1] xfrm_nat_keepalive_repro: delete err=0

-----END crash log-----

Best regards,
Zihan Xi

Zihan Xi (1):
  xfrm: avoid lock inversion in nat keepalive work

 net/xfrm/xfrm_nat_keepalive.c | 57 +++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 9 deletions(-)

-- 
2.43.0