[PATCH bpf v2 0/2] net: xdp: fix bpf_xdp_shrink_data() page handling on generic XDP and veth

[PATCH bpf v2 0/2] net: xdp: fix bpf_xdp_shrink_data() page handling on generic XDP and veth

[Jiayuan Chen]

I'm always confused which tree(net or bpf) should XDP target.

bpf_xdp_shrink_data() frees a page_pool frag with the wrong memory type on
skb-backed XDP, hitting "Bad page state ... page_pool leak". Both the
generic XDP path and the veth path are affected.

Patch 1 fixes it by carrying the memory type in the xdp_buff itself, so it
no longer depends on rxq->mem.type (which is shared on generic XDP and gets
reset on veth). It is reported by syzbot.

Patch 2 adds a selftest that reproduces the leak on both paths.


v1 -> v2: AI found the fix was insufficient and we need a general way
          to fix them.
v1: https://lore.kernel.org/bpf/20260816031245.268898-1-jiayuan.chen@linux.dev/

Jiayuan Chen (2):
  bpf, veth: xdp: fix page_pool page leak on skb-backed XDP
  selftests/bpf: add xdp_shrink_frags

 drivers/net/veth.c                            |   5 +
 include/net/xdp.h                             |  14 ++
 net/core/dev.c                                |   5 +
 net/core/filter.c                             |   7 +
 .../bpf/prog_tests/xdp_shrink_frags.c         | 163 ++++++++++++++++++
 .../selftests/bpf/progs/xdp_shrink_frags.c    |  23 +++
 6 files changed, 217 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c
 create mode 100644 tools/testing/selftests/bpf/progs/xdp_shrink_frags.c

-- 
2.43.0

[PATCH bpf v2 1/2] bpf, veth: xdp: fix page_pool page leak on skb-backed XDP

[Jiayuan Chen]

bpf_xdp_shrink_data() frees a released frag via __xdp_return() using
xdp->rxq->mem.type, but that type is wrong for skb-backed XDP: the skb is
cow'd into page_pool memory while the rxq still says MEM_TYPE_PAGE_SHARED,
so the page_pool page is freed with page_frag_free() and we hit
"Bad page state ... page_pool leak".

Both generic XDP and veth are affected. A non-linear skb is cow'd into
page_pool memory (skb_cow_data_for_xdp() -> skb_pp_cow_data() for generic
XDP, veth_convert_skb_to_xdp_buff() for veth), so its frags become
page_pool pages while the rxq keeps MEM_TYPE_PAGE_SHARED.

We can't just fix rxq->mem.type in place:
- generic XDP: xdp->rxq is dev->_rx[queue].xdp_rxq (see
  bpf_prog_run_generic_xdp()), a shared rxq that other CPUs may access in
  parallel, so we must not write to it.
- veth: rq->xdp_rxq.mem is shared per-queue state that veth resets on XDP
  teardown, and with GRO that reset runs without stopping in-flight NAPI,
  so a type stashed there can be clobbered under a packet still in flight.

Adding a check in __xdp_return() or bpf_xdp_shrink_data() itself is not an
option either: without recording it somewhere, both can only guess the
frag's memory type, which quickly gets confusing.

So record it in the xdp_buff. Add a XDP_FLAGS_FRAGS_PAGE_POOL flag; the two
skb-cow sites set it, and bpf_xdp_shrink_data() frees the frag to the
page_pool when it is set, otherwise it keeps falling back to
xdp->rxq->mem.type unchanged. No other path changes behaviour.

Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic mode")
Fixes: 0ebab78cbcbf ("net: veth: add page_pool for page recycling")
Reported-by: syzbot+237bbeed8dfe0699b7f5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=237bbeed8dfe0699b7f5
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 drivers/net/veth.c |  5 +++++
 include/net/xdp.h  | 14 ++++++++++++++
 net/core/dev.c     |  5 +++++
 net/core/filter.c  |  7 +++++++
 4 files changed, 31 insertions(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 6ed3ee81153f..0afa0661ada1 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -775,6 +775,11 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
 	if (skb_shinfo(skb)->nr_frags) {
 		skb_shinfo(skb)->xdp_frags_size = skb->data_len;
 		xdp_buff_set_frags_flag(xdp);
+		/* A nonlinear skb was cow'd into rq->page_pool above, so the
+		 * frags must be freed to that pool, not via the rxq's
+		 * MEM_TYPE_PAGE_SHARED.
+		 */
+		xdp_buff_set_frag_pp(xdp);
 	} else {
 		xdp_buff_clear_frags_flag(xdp);
 	}
diff --git a/include/net/xdp.h b/include/net/xdp.h
index aa742f413c35..b389dc527adc 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -81,6 +81,10 @@ enum xdp_buff_flags {
 	 * XDP program is not attached.
 	 */
 	XDP_FLAGS_FRAGS_UNREADABLE	= BIT(2),
+	/* frags are page_pool memory even though rxq->mem.type is not: a
+	 * skb-backed XDP buff (generic XDP, veth) is cow'd into a page_pool.
+	 */
+	XDP_FLAGS_FRAGS_PAGE_POOL	= BIT(3),
 };
 
 struct xdp_buff {
@@ -131,6 +135,16 @@ static __always_inline void xdp_buff_set_frag_unreadable(struct xdp_buff *xdp)
 	xdp->flags |= XDP_FLAGS_FRAGS_UNREADABLE;
 }
 
+static __always_inline void xdp_buff_set_frag_pp(struct xdp_buff *xdp)
+{
+	xdp->flags |= XDP_FLAGS_FRAGS_PAGE_POOL;
+}
+
+static __always_inline bool xdp_buff_is_frag_pp(const struct xdp_buff *xdp)
+{
+	return !!(xdp->flags & XDP_FLAGS_FRAGS_PAGE_POOL);
+}
+
 static __always_inline u32 xdp_buff_get_skb_flags(const struct xdp_buff *xdp)
 {
 	return xdp->flags;
diff --git a/net/core/dev.c b/net/core/dev.c
index 38336858c168..be36020484b6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5532,6 +5532,11 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
 	if (skb_is_nonlinear(skb)) {
 		skb_shinfo(skb)->xdp_frags_size = skb->data_len;
 		xdp_buff_set_frags_flag(xdp);
+		/* A nonlinear skb was cow'd into page_pool memory by
+		 * skb_cow_data_for_xdp() before we got here, so the frags must
+		 * be freed to that pool, not via the rxq's MEM_TYPE_PAGE_SHARED.
+		 */
+		xdp_buff_set_frag_pp(xdp);
 	} else {
 		xdp_buff_clear_frags_flag(xdp);
 	}
diff --git a/net/core/filter.c b/net/core/filter.c
index 61940e753552..d34ba56d79d8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4378,6 +4378,13 @@ static bool bpf_xdp_shrink_data(struct xdp_buff *xdp, skb_frag_t *frag,
 	if (mem_type == MEM_TYPE_XSK_BUFF_POOL) {
 		netmem = 0;
 		zc_frag = bpf_xdp_shrink_data_zc(xdp, shrink, tail, release);
+	} else if (xdp_buff_is_frag_pp(xdp)) {
+		/*
+		 * Skb-backed XDP (generic XDP, veth) cow's the frags into a
+		 * page_pool while the rxq stays MEM_TYPE_PAGE_SHARED, so free
+		 * the frag to the pool, not via page_frag_free().
+		 */
+		mem_type = MEM_TYPE_PAGE_POOL;
 	}
 
 	if (release) {
-- 
2.43.0

[PATCH bpf v2 2/2] selftests/bpf: add xdp_shrink_frags

[Jiayuan Chen]

Add a test that attaches an xdp.frags program which shrinks a whole frag
away, so bpf_xdp_shrink_data() frees a page_pool frag.

test_tun triggers the page-type mismatch on the generic XDP path.
test_veth triggers the same mismatch on the veth path.

Both reproduce "Bad page state ... page_pool leak" on a buggy kernel.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 .../bpf/prog_tests/xdp_shrink_frags.c         | 163 ++++++++++++++++++
 .../selftests/bpf/progs/xdp_shrink_frags.c    |  23 +++
 2 files changed, 186 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c
 create mode 100644 tools/testing/selftests/bpf/progs/xdp_shrink_frags.c

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c
new file mode 100644
index 000000000000..f3d8a84a6dc9
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <network_helpers.h>
+#include <linux/if_tun.h>
+#include <linux/if_ether.h>
+#include <sys/uio.h>
+#include <net/if.h>
+#include <arpa/inet.h>
+#include "xdp_shrink_frags.skel.h"
+
+/*
+ * A generic-XDP program that shrinks into the frags frees a page_pool frag.
+ * skb-backed XDP first cow's the nonlinear skb into page_pool memory
+ * (skb_cow_data_for_xdp() for generic XDP, skb_pp_cow_data() for veth), but
+ * the shared rxq is registered as MEM_TYPE_PAGE_SHARED, so a buggy kernel
+ * frees the frag with page_frag_free() -> "Bad page state ... page_pool leak".
+ */
+
+#define TAP_NAME	"xdp_shrink0"
+#define TAP_NETNS	"xdp_shrink_tap"
+
+#define VETH_LOCAL	"xdp_shrinkA"
+#define VETH_PEER	"xdp_shrinkB"
+#define VETH_NETNS	"xdp_shrink_veth"
+#define VETH_LOCAL_IP	"10.9.9.1"
+#define VETH_PEER_IP	"10.9.9.2"
+
+static int create_tap_napi_frags(const char *ifname)
+{
+	struct ifreq ifr = {
+		.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_NAPI | IFF_NAPI_FRAGS,
+	};
+	int fd, err;
+
+	strscpy(ifr.ifr_name, ifname);
+
+	fd = open("/dev/net/tun", O_RDWR);
+	if (fd < 0)
+		return -1;
+
+	err = ioctl(fd, TUNSETIFF, &ifr);
+	if (err) {
+		close(fd);
+		return -1;
+	}
+
+	return fd;
+}
+
+/*
+ * Similar to flow_dissector.c: writev() an IFF_NAPI_FRAGS tap to build a
+ * nonlinear skb (sized for 4K pages, like xdp_adjust_tail.c) that tun runs
+ * through do_xdp_generic().
+ */
+static void test_tun(struct xdp_shrink_frags *skel)
+{
+	__u8 head[74], frag1[2048], frag2[2048];
+	struct ethhdr *eth = (void *)head;
+	int tap_fd = -1, ifindex, err;
+	struct netns_obj *ns = NULL;
+	struct iovec iov[3];
+	ssize_t n;
+
+	ns = netns_new(TAP_NETNS, true);
+	if (!ASSERT_OK_PTR(ns, "netns_new"))
+		return;
+
+	tap_fd = create_tap_napi_frags(TAP_NAME);
+	if (!ASSERT_GE(tap_fd, 0, "create_tap"))
+		goto out;
+
+	SYS(out, "ip link set dev " TAP_NAME " up");
+
+	ifindex = if_nametoindex(TAP_NAME);
+	if (!ASSERT_GT(ifindex, 0, "if_nametoindex"))
+		goto out;
+
+	skel->bss->shrink_ran = 0;
+
+	err = bpf_xdp_attach(ifindex, bpf_program__fd(skel->progs.xdp_shrink),
+			     0, NULL);
+	if (!ASSERT_OK(err, "bpf_xdp_attach"))
+		goto out;
+
+	memset(head, 0, sizeof(head));
+	memset(frag1, 0x41, sizeof(frag1));
+	memset(frag2, 0x42, sizeof(frag2));
+	eth->h_proto = htons(ETH_P_IP);
+
+	iov[0].iov_base = head;  iov[0].iov_len = sizeof(head);
+	iov[1].iov_base = frag1; iov[1].iov_len = sizeof(frag1);
+	iov[2].iov_base = frag2; iov[2].iov_len = sizeof(frag2);
+
+	n = writev(tap_fd, iov, ARRAY_SIZE(iov));
+	ASSERT_EQ(n, sizeof(head) + sizeof(frag1) + sizeof(frag2), "writev");
+
+	usleep(100 * 1000);
+	ASSERT_GT(skel->bss->shrink_ran, 0, "xdp_prog_ran");
+
+	bpf_xdp_detach(ifindex, 0, NULL);
+out:
+	if (tap_fd >= 0)
+		close(tap_fd);
+	netns_free(ns);
+}
+
+/*
+ * A large ping builds a nonlinear skb that veth cow's into its page_pool
+ * (sized for 4K pages, like xdp_adjust_tail.c) before running the program.
+ */
+static void test_veth(struct xdp_shrink_frags *skel)
+{
+	int ifindex, err;
+
+	SYS(out, "ip netns add " VETH_NETNS);
+	SYS(out_ns, "ip link add %s mtu 8000 type veth peer name %s mtu 8000",
+	    VETH_LOCAL, VETH_PEER);
+	SYS(out_link, "ip link set " VETH_PEER " netns " VETH_NETNS);
+	SYS(out_link, "ip addr add " VETH_LOCAL_IP "/24 dev " VETH_LOCAL);
+	SYS(out_link, "ip link set " VETH_LOCAL " up");
+	SYS(out_link, "ip -n " VETH_NETNS " addr add " VETH_PEER_IP "/24 dev " VETH_PEER);
+	SYS(out_link, "ip -n " VETH_NETNS " link set " VETH_PEER " up");
+
+	ifindex = if_nametoindex(VETH_LOCAL);
+	if (!ASSERT_GT(ifindex, 0, "if_nametoindex"))
+		goto out_link;
+
+	skel->bss->shrink_ran = 0;
+
+	err = bpf_xdp_attach(ifindex, bpf_program__fd(skel->progs.xdp_shrink),
+			     0, NULL);
+	if (!ASSERT_OK(err, "bpf_xdp_attach"))
+		goto out_link;
+
+	SYS_NOFAIL("ip netns exec " VETH_NETNS
+		   " ping -q -s 5000 -c 3 -W 1 " VETH_LOCAL_IP);
+
+	ASSERT_GT(skel->bss->shrink_ran, 0, "xdp_prog_ran");
+
+	bpf_xdp_detach(ifindex, 0, NULL);
+out_link:
+	SYS_NOFAIL("ip link del " VETH_LOCAL);
+out_ns:
+	SYS_NOFAIL("ip netns del " VETH_NETNS);
+out:
+	return;
+}
+
+void test_xdp_shrink_frags(void)
+{
+	struct xdp_shrink_frags *skel;
+
+	skel = xdp_shrink_frags__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open_load"))
+		return;
+
+	if (test__start_subtest("tun"))
+		test_tun(skel);
+	if (test__start_subtest("veth"))
+		test_veth(skel);
+
+	xdp_shrink_frags__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c b/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c
new file mode 100644
index 000000000000..62e3791ac81d
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+int shrink_ran;
+
+SEC("xdp.frags")
+int xdp_shrink(struct xdp_md *ctx)
+{
+	/*
+	 * The program is loaded with BPF_F_XDP_HAS_FRAGS (xdp.frags), so a
+	 * nonlinear skb entering generic XDP is cow'd into page_pool memory
+	 * before we run. Shrinking the tail far enough releases at least one
+	 * whole frag, which must be returned to its page_pool. Count only a
+	 * successful shrink so a too-small frame (e.g. ARP) does not satisfy
+	 * the test.
+	 */
+	if (bpf_xdp_adjust_tail(ctx, -3000) == 0)
+		__sync_fetch_and_add(&shrink_ran, 1);
+	return XDP_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.43.0

Re: [PATCH bpf v2 2/2] selftests/bpf: add xdp_shrink_frags

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 10216 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c
> new file mode 100644
> index 000000000000..f3d8a84a6dc9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c
> @@ -0,0 +1,163 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <network_helpers.h>
> +#include <linux/if_tun.h>
> +#include <linux/if_ether.h>
> +#include <sys/uio.h>
> +#include <net/if.h>
> +#include <arpa/inet.h>
> +#include "xdp_shrink_frags.skel.h"
> +
> +/*
> + * A generic-XDP program that shrinks into the frags frees a page_pool frag.
> + * skb-backed XDP first cow's the nonlinear skb into page_pool memory
> + * (skb_cow_data_for_xdp() for generic XDP, skb_pp_cow_data() for veth), but
> + * the shared rxq is registered as MEM_TYPE_PAGE_SHARED, so a buggy kernel
> + * frees the frag with page_frag_free() -> "Bad page state ... page_pool leak".
> + */
> +
> +#define TAP_NAME	"xdp_shrink0"
> +#define TAP_NETNS	"xdp_shrink_tap"
> +
> +#define VETH_LOCAL	"xdp_shrinkA"
> +#define VETH_PEER	"xdp_shrinkB"
> +#define VETH_NETNS	"xdp_shrink_veth"
> +#define VETH_LOCAL_IP	"10.9.9.1"
> +#define VETH_PEER_IP	"10.9.9.2"
> +
> +static int create_tap_napi_frags(const char *ifname)
> +{
> +	struct ifreq ifr = {
> +		.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_NAPI | IFF_NAPI_FRAGS,
> +	};
> +	int fd, err;
> +
> +	strscpy(ifr.ifr_name, ifname);
> +
> +	fd = open("/dev/net/tun", O_RDWR);
> +	if (fd < 0)
> +		return -1;

A subsystem pattern flags this as potentially concerning:
create_tap_napi_frags() collapses every failure into -1 and discards
errno, so the caller's ASSERT_GE(tap_fd, 0, "create_tap") becomes a hard
test failure with no diagnostic.

Two concrete cases where this would happen: /dev/net/tun missing
(CONFIG_TUN not built or the node absent) gives ENOENT/ENODEV, and
TUNSETIFF with IFF_NAPI_FRAGS rejected with EPERM when CAP_NET_ADMIN is
not held in the initial user namespace. Both of these should skip the test
with a reason rather than fail it. At minimum, would keeping errno make the
failure diagnosable?

> +
> +	err = ioctl(fd, TUNSETIFF, &ifr);
> +	if (err) {
> +		close(fd);
> +		return -1;
> +	}
> +
> +	return fd;
> +}
> +
> +/*
> + * Similar to flow_dissector.c: writev() an IFF_NAPI_FRAGS tap to build a
> + * nonlinear skb (sized for 4K pages, like xdp_adjust_tail.c) that tun runs
> + * through do_xdp_generic().
> + */
> +static void test_tun(struct xdp_shrink_frags *skel)
> +{
> +	__u8 head[74], frag1[2048], frag2[2048];
> +	struct ethhdr *eth = (void *)head;
> +	int tap_fd = -1, ifindex, err;
> +	struct netns_obj *ns = NULL;
> +	struct iovec iov[3];
> +	ssize_t n;
> +
> +	ns = netns_new(TAP_NETNS, true);
> +	if (!ASSERT_OK_PTR(ns, "netns_new"))
> +		return;
> +
> +	tap_fd = create_tap_napi_frags(TAP_NAME);
> +	if (!ASSERT_GE(tap_fd, 0, "create_tap"))
> +		goto out;
> +
> +	SYS(out, "ip link set dev " TAP_NAME " up");
> +
> +	ifindex = if_nametoindex(TAP_NAME);
> +	if (!ASSERT_GT(ifindex, 0, "if_nametoindex"))
> +		goto out;
> +
> +	skel->bss->shrink_ran = 0;
> +
> +	err = bpf_xdp_attach(ifindex, bpf_program__fd(skel->progs.xdp_shrink),
> +			     0, NULL);
> +	if (!ASSERT_OK(err, "bpf_xdp_attach"))
> +		goto out;
> +
> +	memset(head, 0, sizeof(head));
> +	memset(frag1, 0x41, sizeof(frag1));
> +	memset(frag2, 0x42, sizeof(frag2));
> +	eth->h_proto = htons(ETH_P_IP);
> +
> +	iov[0].iov_base = head;  iov[0].iov_len = sizeof(head);
> +	iov[1].iov_base = frag1; iov[1].iov_len = sizeof(frag1);
> +	iov[2].iov_base = frag2; iov[2].iov_len = sizeof(frag2);
> +
> +	n = writev(tap_fd, iov, ARRAY_SIZE(iov));
> +	ASSERT_EQ(n, sizeof(head) + sizeof(frag1) + sizeof(frag2), "writev");
> +
> +	usleep(100 * 1000);
> +	ASSERT_GT(skel->bss->shrink_ran, 0, "xdp_prog_ran");

Does this test actually exercise the page_pool frag release it is written
for? The frame sizes (74 + 2048 + 2048 = 4170 bytes) are picked for 4K
pages, but skb_pp_cow_data() puts up to SKB_WITH_OVERHEAD(PAGE_SIZE -
headroom) bytes in the linear head.

On 4K pages with XDP_PACKET_HEADROOM (256), that's ~3520 bytes in the
head, so the 4170-byte packet gets frags.

But on 64K pages, the threshold becomes ~64960 bytes, so the entire
4170-byte packet ends up linear and bpf_xdp_adjust_tail(-3000) never
reaches bpf_xdp_shrink_data() - it just moves data_end and returns 0.
shrink_ran still gets incremented, so the ASSERT_GT() passes whether or
not the fix from the preceding commit is applied.

selftests/bpf ships config.ppc64el and config.aarch64, and the existing
xdp_adjust_tail.c that the sizing comments cite explicitly branches on
getpagesize() == 65536. Would gating on getpagesize() or asserting
something that only holds when a whole frag was released (packet length or
a frag count observed from the program) make the test discriminate a fixed
kernel from a buggy one on 64K-page systems?

> +
> +	bpf_xdp_detach(ifindex, 0, NULL);

[ ... ]

> +/*
> + * A large ping builds a nonlinear skb that veth cow's into its page_pool
> + * (sized for 4K pages, like xdp_adjust_tail.c) before running the program.
> + */
> +static void test_veth(struct xdp_shrink_frags *skel)
> +{
> +	int ifindex, err;
> +
> +	SYS(out, "ip netns add " VETH_NETNS);
> +	SYS(out_ns, "ip link add %s mtu 8000 type veth peer name %s mtu 8000",
> +	    VETH_LOCAL, VETH_PEER);

A subsystem pattern flags this as potentially concerning: test_veth
hand-rolls namespace management with `ip netns add` / `ip netns del`
instead of the shared netns_new()/netns_free() helpers that test_tun uses
50 lines earlier in the same file.

It also only puts the peer into a namespace - the local end xdp_shrinkA
and its address 10.9.9.1/24 are created in whatever namespace test_progs
is running in (normally the host), so the test mutates the caller's
network configuration. The cleanup is best-effort SYS_NOFAIL, leaving the
device and address behind if the process dies between setup and teardown.
Fixed device/netns names also mean two concurrent test_progs workers
collide.

prog_tests/test_xdp_veth.c in the same directory keeps even the "local"
side inside a dedicated ns0 namespace for exactly this reason, and appends
the TID to the name. Would wrapping test_veth in netns_new() for a local
namespace and creating both ends inside it make the netns teardown remove
the veth pair, the address and the XDP program together?

> +	SYS(out_link, "ip link set " VETH_PEER " netns " VETH_NETNS);
> +	SYS(out_link, "ip addr add " VETH_LOCAL_IP "/24 dev " VETH_LOCAL);
> +	SYS(out_link, "ip link set " VETH_LOCAL " up");
> +	SYS(out_link, "ip -n " VETH_NETNS " addr add " VETH_PEER_IP "/24 dev " VETH_PEER);
> +	SYS(out_link, "ip -n " VETH_NETNS " link set " VETH_PEER " up");
> +
> +	ifindex = if_nametoindex(VETH_LOCAL);
> +	if (!ASSERT_GT(ifindex, 0, "if_nametoindex"))
> +		goto out_link;
> +
> +	skel->bss->shrink_ran = 0;
> +
> +	err = bpf_xdp_attach(ifindex, bpf_program__fd(skel->progs.xdp_shrink),
> +			     0, NULL);
> +	if (!ASSERT_OK(err, "bpf_xdp_attach"))
> +		goto out_link;
> +
> +	SYS_NOFAIL("ip netns exec " VETH_NETNS
> +		   " ping -q -s 5000 -c 3 -W 1 " VETH_LOCAL_IP);
> +
> +	ASSERT_GT(skel->bss->shrink_ran, 0, "xdp_prog_ran");

This test has the same page-size dependency as test_tun above. The `ping
-s 5000` produces ~5042 bytes on the wire, which is below the ~64960-byte
linear threshold on 64K-page kernels, so veth_convert_skb_to_xdp_buff()
sees nr_frags == 0 and bpf_xdp_adjust_tail() never enters
bpf_xdp_shrink_data().

> +
> +	bpf_xdp_detach(ifindex, 0, NULL);

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c b/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c
> new file mode 100644
> index 000000000000..62e3791ac81d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +
> +int shrink_ran;
> +
> +SEC("xdp.frags")
> +int xdp_shrink(struct xdp_md *ctx)
> +{
> +	/*
> +	 * The program is loaded with BPF_F_XDP_HAS_FRAGS (xdp.frags), so a
> +	 * nonlinear skb entering generic XDP is cow'd into page_pool memory
> +	 * before we run. Shrinking the tail far enough releases at least one
> +	 * whole frag, which must be returned to its page_pool. Count only a
> +	 * successful shrink so a too-small frame (e.g. ARP) does not satisfy
> +	 * the test.
> +	 */

Does this comment accurately describe what the test guarantees? "Shrinking
the tail far enough releases at least one whole frag" only holds if the
cow'd skb has frags at all.

skb_pp_cow_data() puts up to SKB_WITH_OVERHEAD(PAGE_SIZE - headroom) bytes
in the linear head:

net/core/skbuff.c:skb_pp_cow_data() {
    max_head_size = SKB_WITH_OVERHEAD(PAGE_SIZE - headroom);
    size = min_t(u32, skb->len, max_head_size);
    ...
    for (i = 0; i < MAX_SKB_FRAGS && off < skb->len; i++)
}

With PAGE_SIZE == 65536 and headroom == XDP_PACKET_HEADROOM (256),
max_head_size is ~64960 bytes. The test's frame sizes are 4170 bytes (tun)
and ~5042 bytes (veth), so on a 64K-page kernel there are zero frags and
bpf_xdp_adjust_tail() still returns 0 without calling bpf_xdp_shrink_data().

Two smaller mismatches in the same comment block and in the prog_tests
header comment:
  - This program is attached on both the generic-XDP (tun) and veth paths,
    but the comment only describes "a nonlinear skb entering generic XDP".
  - The prog_tests header says "skb_cow_data_for_xdp() for generic XDP,
    skb_pp_cow_data() for veth", which reads as two different mechanisms.
    But skb_cow_data_for_xdp() is just a wrapper that calls
    skb_pp_cow_data(), and the veth call site is
    veth_convert_skb_to_xdp_buff() - both paths end up in skb_pp_cow_data().

> +	if (bpf_xdp_adjust_tail(ctx, -3000) == 0)
> +		__sync_fetch_and_add(&shrink_ran, 1);
> +	return XDP_PASS;
> +}
> +
> +char _license[] SEC("license") = "GPL";

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32686145995

Re: [PATCH bpf v2 2/2] selftests/bpf: add xdp_shrink_frags

[Jiayuan Chen]

On 8/24/26 11:58 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c
>> new file mode 100644
>> index 000000000000..f3d8a84a6dc9
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c
>> @@ -0,0 +1,163 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <test_progs.h>
>> +#include <network_helpers.h>
>> +#include <linux/if_tun.h>
>> +#include <linux/if_ether.h>
>> +#include <sys/uio.h>
>> +#include <net/if.h>
>> +#include <arpa/inet.h>
>> +#include "xdp_shrink_frags.skel.h"
>> +
>> +/*
>> + * A generic-XDP program that shrinks into the frags frees a page_pool frag.
>> + * skb-backed XDP first cow's the nonlinear skb into page_pool memory
>> + * (skb_cow_data_for_xdp() for generic XDP, skb_pp_cow_data() for veth), but
>> + * the shared rxq is registered as MEM_TYPE_PAGE_SHARED, so a buggy kernel
>> + * frees the frag with page_frag_free() -> "Bad page state ... page_pool leak".
>> + */
>> +
>> +#define TAP_NAME	"xdp_shrink0"
>> +#define TAP_NETNS	"xdp_shrink_tap"
>> +
>> +#define VETH_LOCAL	"xdp_shrinkA"
>> +#define VETH_PEER	"xdp_shrinkB"
>> +#define VETH_NETNS	"xdp_shrink_veth"
>> +#define VETH_LOCAL_IP	"10.9.9.1"
>> +#define VETH_PEER_IP	"10.9.9.2"
>> +
>> +static int create_tap_napi_frags(const char *ifname)
>> +{
>> +	struct ifreq ifr = {
>> +		.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_NAPI | IFF_NAPI_FRAGS,
>> +	};
>> +	int fd, err;
>> +
>> +	strscpy(ifr.ifr_name, ifname);
>> +
>> +	fd = open("/dev/net/tun", O_RDWR);
>> +	if (fd < 0)
>> +		return -1;
> A subsystem pattern flags this as potentially concerning:
> create_tap_napi_frags() collapses every failure into -1 and discards
> errno, so the caller's ASSERT_GE(tap_fd, 0, "create_tap") becomes a hard
> test failure with no diagnostic.
>
> Two concrete cases where this would happen: /dev/net/tun missing
> (CONFIG_TUN not built or the node absent) gives ENOENT/ENODEV, and
> TUNSETIFF with IFF_NAPI_FRAGS rejected with EPERM when CAP_NET_ADMIN is
> not held in the initial user namespace. Both of these should skip the test
> with a reason rather than fail it. At minimum, would keeping errno make the
> failure diagnosable?


flow_dissector.c's create_tap() (which this is modeled on) does exactly 
the same and doesn't even close the fd on the ioctl failure.

so I'd rather keep the parity than diverge here.

>
>> +
>> +	err = ioctl(fd, TUNSETIFF, &ifr);
>> +	if (err) {
>> +		close(fd);
>> +		return -1;
>> +	}
>> +
>> +	return fd;
>> +}
>> +
>> +/*
>> + * Similar to flow_dissector.c: writev() an IFF_NAPI_FRAGS tap to build a
>> + * nonlinear skb (sized for 4K pages, like xdp_adjust_tail.c) that tun runs
>> + * through do_xdp_generic().
>> + */
>> +static void test_tun(struct xdp_shrink_frags *skel)
>> +{
>> +	__u8 head[74], frag1[2048], frag2[2048];
>> +	struct ethhdr *eth = (void *)head;
>> +	int tap_fd = -1, ifindex, err;
>> +	struct netns_obj *ns = NULL;
>> +	struct iovec iov[3];
>> +	ssize_t n;
>> +
>> +	ns = netns_new(TAP_NETNS, true);
>> +	if (!ASSERT_OK_PTR(ns, "netns_new"))
>> +		return;
>> +
>> +	tap_fd = create_tap_napi_frags(TAP_NAME);
>> +	if (!ASSERT_GE(tap_fd, 0, "create_tap"))
>> +		goto out;
>> +
>> +	SYS(out, "ip link set dev " TAP_NAME " up");
>> +
>> +	ifindex = if_nametoindex(TAP_NAME);
>> +	if (!ASSERT_GT(ifindex, 0, "if_nametoindex"))
>> +		goto out;
>> +
>> +	skel->bss->shrink_ran = 0;
>> +
>> +	err = bpf_xdp_attach(ifindex, bpf_program__fd(skel->progs.xdp_shrink),
>> +			     0, NULL);
>> +	if (!ASSERT_OK(err, "bpf_xdp_attach"))
>> +		goto out;
>> +
>> +	memset(head, 0, sizeof(head));
>> +	memset(frag1, 0x41, sizeof(frag1));
>> +	memset(frag2, 0x42, sizeof(frag2));
>> +	eth->h_proto = htons(ETH_P_IP);
>> +
>> +	iov[0].iov_base = head;  iov[0].iov_len = sizeof(head);
>> +	iov[1].iov_base = frag1; iov[1].iov_len = sizeof(frag1);
>> +	iov[2].iov_base = frag2; iov[2].iov_len = sizeof(frag2);
>> +
>> +	n = writev(tap_fd, iov, ARRAY_SIZE(iov));
>> +	ASSERT_EQ(n, sizeof(head) + sizeof(frag1) + sizeof(frag2), "writev");
>> +
>> +	usleep(100 * 1000);
>> +	ASSERT_GT(skel->bss->shrink_ran, 0, "xdp_prog_ran");
> Does this test actually exercise the page_pool frag release it is written
> for? The frame sizes (74 + 2048 + 2048 = 4170 bytes) are picked for 4K
> pages, but skb_pp_cow_data() puts up to SKB_WITH_OVERHEAD(PAGE_SIZE -
> headroom) bytes in the linear head.
>
> On 4K pages with XDP_PACKET_HEADROOM (256), that's ~3520 bytes in the
> head, so the 4170-byte packet gets frags.
>
> But on 64K pages, the threshold becomes ~64960 bytes, so the entire
> 4170-byte packet ends up linear and bpf_xdp_adjust_tail(-3000) never
> reaches bpf_xdp_shrink_data() - it just moves data_end and returns 0.
> shrink_ran still gets incremented, so the ASSERT_GT() passes whether or
> not the fix from the preceding commit is applied.
>
> selftests/bpf ships config.ppc64el and config.aarch64, and the existing
> xdp_adjust_tail.c that the sizing comments cite explicitly branches on
> getpagesize() == 65536. Would gating on getpagesize() or asserting
> something that only holds when a whole frag was released (packet length or
> a frag count observed from the program) make the test discriminate a fixed
> kernel from a buggy one on 64K-page systems?


On 64K it just passes as a no-op and never false-fails, and the 
analogous in-tree test test_xdp_adjust_frags_tail_shrink() also hardcodes 4K

and runs unconditionally, so this keeps parity with it (and BPF CI runs 
4K-page kernels anyway).

>> +
>> +	bpf_xdp_detach(ifindex, 0, NULL);
> [ ... ]
>
>> +/*
>> + * A large ping builds a nonlinear skb that veth cow's into its page_pool
>> + * (sized for 4K pages, like xdp_adjust_tail.c) before running the program.
>> + */
>> +static void test_veth(struct xdp_shrink_frags *skel)
>> +{
>> +	int ifindex, err;
>> +
>> +	SYS(out, "ip netns add " VETH_NETNS);
>> +	SYS(out_ns, "ip link add %s mtu 8000 type veth peer name %s mtu 8000",
>> +	    VETH_LOCAL, VETH_PEER);
> A subsystem pattern flags this as potentially concerning: test_veth
> hand-rolls namespace management with `ip netns add` / `ip netns del`
> instead of the shared netns_new()/netns_free() helpers that test_tun uses
> 50 lines earlier in the same file.
>
> It also only puts the peer into a namespace - the local end xdp_shrinkA
> and its address 10.9.9.1/24 are created in whatever namespace test_progs
> is running in (normally the host), so the test mutates the caller's
> network configuration. The cleanup is best-effort SYS_NOFAIL, leaving the
> device and address behind if the process dies between setup and teardown.
> Fixed device/netns names also mean two concurrent test_progs workers
> collide.
>
> prog_tests/test_xdp_veth.c in the same directory keeps even the "local"
> side inside a dedicated ns0 namespace for exactly this reason, and appends
> the TID to the name. Would wrapping test_veth in netns_new() for a local
> namespace and creating both ends inside it make the netns teardown remove
> the veth pair, the address and the XDP program together?


  Sounds reasonable. I'll wrap test_veth in netns_new() and create both 
veth ends inside it, like test_tun does.


>
>> +	SYS(out_link, "ip link set " VETH_PEER " netns " VETH_NETNS);
>> +	SYS(out_link, "ip addr add " VETH_LOCAL_IP "/24 dev " VETH_LOCAL);
>> +	SYS(out_link, "ip link set " VETH_LOCAL " up");
>> +	SYS(out_link, "ip -n " VETH_NETNS " addr add " VETH_PEER_IP "/24 dev " VETH_PEER);
>> +	SYS(out_link, "ip -n " VETH_NETNS " link set " VETH_PEER " up");
>> +
>> +	ifindex = if_nametoindex(VETH_LOCAL);
>> +	if (!ASSERT_GT(ifindex, 0, "if_nametoindex"))
>> +		goto out_link;
>> +
>> +	skel->bss->shrink_ran = 0;
>> +
>> +	err = bpf_xdp_attach(ifindex, bpf_program__fd(skel->progs.xdp_shrink),
>> +			     0, NULL);
>> +	if (!ASSERT_OK(err, "bpf_xdp_attach"))
>> +		goto out_link;
>> +
>> +	SYS_NOFAIL("ip netns exec " VETH_NETNS
>> +		   " ping -q -s 5000 -c 3 -W 1 " VETH_LOCAL_IP);
>> +
>> +	ASSERT_GT(skel->bss->shrink_ran, 0, "xdp_prog_ran");
> This test has the same page-size dependency as test_tun above. The `ping
> -s 5000` produces ~5042 bytes on the wire, which is below the ~64960-byte
> linear threshold on 64K-page kernels, so veth_convert_skb_to_xdp_buff()
> sees nr_frags == 0 and bpf_xdp_adjust_tail() never enters
> bpf_xdp_shrink_data().


Same as the tun one: no false-fail on 64K, and CI is 4K.


>> +
>> +	bpf_xdp_detach(ifindex, 0, NULL);
> [ ... ]
>
>> diff --git a/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c b/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c
>> new file mode 100644
>> index 000000000000..62e3791ac81d
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c
>> @@ -0,0 +1,23 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include "vmlinux.h"
>> +#include <bpf/bpf_helpers.h>
>> +
>> +int shrink_ran;
>> +
>> +SEC("xdp.frags")
>> +int xdp_shrink(struct xdp_md *ctx)
>> +{
>> +	/*
>> +	 * The program is loaded with BPF_F_XDP_HAS_FRAGS (xdp.frags), so a
>> +	 * nonlinear skb entering generic XDP is cow'd into page_pool memory
>> +	 * before we run. Shrinking the tail far enough releases at least one
>> +	 * whole frag, which must be returned to its page_pool. Count only a
>> +	 * successful shrink so a too-small frame (e.g. ARP) does not satisfy
>> +	 * the test.
>> +	 */
> Does this comment accurately describe what the test guarantees? "Shrinking
> the tail far enough releases at least one whole frag" only holds if the
> cow'd skb has frags at all.
>
> skb_pp_cow_data() puts up to SKB_WITH_OVERHEAD(PAGE_SIZE - headroom) bytes
> in the linear head:
>
> net/core/skbuff.c:skb_pp_cow_data() {
>      max_head_size = SKB_WITH_OVERHEAD(PAGE_SIZE - headroom);
>      size = min_t(u32, skb->len, max_head_size);
>      ...
>      for (i = 0; i < MAX_SKB_FRAGS && off < skb->len; i++)
> }
>
> With PAGE_SIZE == 65536 and headroom == XDP_PACKET_HEADROOM (256),
> max_head_size is ~64960 bytes. The test's frame sizes are 4170 bytes (tun)
> and ~5042 bytes (veth), so on a 64K-page kernel there are zero frags and
> bpf_xdp_adjust_tail() still returns 0 without calling bpf_xdp_shrink_data().


On the 4K pages the test targets (like the other frags tests) it does 
have frags,

so the comment holds for what the test actually runs.


> Two smaller mismatches in the same comment block and in the prog_tests
> header comment:
>    - This program is attached on both the generic-XDP (tun) and veth paths,
>      but the comment only describes "a nonlinear skb entering generic XDP".

The file header already says it covers both generic XDP and veth;

the prog comment just uses the generic path as the example and the cow 
behaviour is identical.

>    - The prog_tests header says "skb_cow_data_for_xdp() for generic XDP,
>      skb_pp_cow_data() for veth", which reads as two different mechanisms.
>      But skb_cow_data_for_xdp() is just a wrapper that calls
>      skb_pp_cow_data(), and the veth call site is
>      veth_convert_skb_to_xdp_buff() - both paths end up in skb_pp_cow_data().


Those are the actual call sites (generic goes through 
skb_cow_data_for_xdp(), veth calls skb_pp_cow_data()

directly from veth_convert_skb_to_xdp_buff()), so naming both is 
accurate rather than two different mechanisms.

>
>> +	if (bpf_xdp_adjust_tail(ctx, -3000) == 0)
>> +		__sync_fetch_and_add(&shrink_ran, 1);
>> +	return XDP_PASS;
>> +}
>> +
>> +char _license[] SEC("license") = "GPL";
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32686145995

Re: [PATCH bpf v2 1/2] bpf, veth: xdp: fix page_pool page leak on skb-backed XDP

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 6335 bytes --]

> bpf_xdp_shrink_data() frees a released frag via __xdp_return() using
> xdp->rxq->mem.type, but that type is wrong for skb-backed XDP: the skb is
> cow'd into page_pool memory while the rxq still says MEM_TYPE_PAGE_SHARED,
> so the page_pool page is freed with page_frag_free() and we hit
> "Bad page state ... page_pool leak".
> 
> Both generic XDP and veth are affected. A non-linear skb is cow'd into
> page_pool memory (skb_cow_data_for_xdp() -> skb_pp_cow_data() for generic
> XDP, veth_convert_skb_to_xdp_buff() for veth), so its frags become
> page_pool pages while the rxq keeps MEM_TYPE_PAGE_SHARED.
> 
> We can't just fix rxq->mem.type in place:
> - generic XDP: xdp->rxq is dev->_rx[queue].xdp_rxq (see
>   bpf_prog_run_generic_xdp()), a shared rxq that other CPUs may access in
>   parallel, so we must not write to it.
> - veth: rq->xdp_rxq.mem is shared per-queue state that veth resets on XDP
>   teardown, and with GRO that reset runs without stopping in-flight NAPI,
>   so a type stashed there can be clobbered under a packet still in flight.
> 
> Adding a check in __xdp_return() or bpf_xdp_shrink_data() itself is not an
> option either: without recording it somewhere, both can only guess the
> frag's memory type, which quickly gets confusing.
> 
> So record it in the xdp_buff. Add a XDP_FLAGS_FRAGS_PAGE_POOL flag; the two
> skb-cow sites set it, and bpf_xdp_shrink_data() frees the frag to the
> page_pool when it is set, otherwise it keeps falling back to
> xdp->rxq->mem.type unchanged. No other path changes behaviour.
> 
> Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic mode")
> Fixes: 0ebab78cbcbf ("net: veth: add page_pool for page recycling")

Hi Jiayuan Chen,

thx for fixing it. Can we do something like the patch below instead?

Regards,
Lorenzo

diff --git a/net/core/xdp.c b/net/core/xdp.c
index 1d679e8fd649..4ed659b58141 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -433,16 +433,16 @@ EXPORT_SYMBOL_GPL(xdp_rxq_info_attach_page_pool);
 void __xdp_return(netmem_ref netmem, enum xdp_mem_type mem_type,
 		  bool napi_direct, struct xdp_buff *xdp)
 {
+	netmem_ref head_netmem = netmem_compound_head(netmem);
+	if (netmem_is_pp(head_netmem))
+		mem_type = MEM_TYPE_PAGE_POOL;
+
 	switch (mem_type) {
 	case MEM_TYPE_PAGE_POOL:
-		netmem = netmem_compound_head(netmem);
 		if (napi_direct && xdp_return_frame_no_direct())
 			napi_direct = false;
-		/* No need to check netmem_is_pp() as mem->type knows this a
-		 * page_pool page
-		 */
-		page_pool_put_full_netmem(netmem_get_pp(netmem), netmem,
-					  napi_direct);
+		page_pool_put_full_netmem(netmem_get_pp(head_netmem),
+					  head_netmem, napi_direct);
 		break;
 	case MEM_TYPE_PAGE_SHARED:
 		page_frag_free(__netmem_address(netmem));


> Reported-by: syzbot+237bbeed8dfe0699b7f5@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=237bbeed8dfe0699b7f5
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
>  drivers/net/veth.c |  5 +++++
>  include/net/xdp.h  | 14 ++++++++++++++
>  net/core/dev.c     |  5 +++++
>  net/core/filter.c  |  7 +++++++
>  4 files changed, 31 insertions(+)
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 6ed3ee81153f..0afa0661ada1 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -775,6 +775,11 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
>  	if (skb_shinfo(skb)->nr_frags) {
>  		skb_shinfo(skb)->xdp_frags_size = skb->data_len;
>  		xdp_buff_set_frags_flag(xdp);
> +		/* A nonlinear skb was cow'd into rq->page_pool above, so the
> +		 * frags must be freed to that pool, not via the rxq's
> +		 * MEM_TYPE_PAGE_SHARED.
> +		 */
> +		xdp_buff_set_frag_pp(xdp);
>  	} else {
>  		xdp_buff_clear_frags_flag(xdp);
>  	}
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index aa742f413c35..b389dc527adc 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -81,6 +81,10 @@ enum xdp_buff_flags {
>  	 * XDP program is not attached.
>  	 */
>  	XDP_FLAGS_FRAGS_UNREADABLE	= BIT(2),
> +	/* frags are page_pool memory even though rxq->mem.type is not: a
> +	 * skb-backed XDP buff (generic XDP, veth) is cow'd into a page_pool.
> +	 */
> +	XDP_FLAGS_FRAGS_PAGE_POOL	= BIT(3),
>  };
>  
>  struct xdp_buff {
> @@ -131,6 +135,16 @@ static __always_inline void xdp_buff_set_frag_unreadable(struct xdp_buff *xdp)
>  	xdp->flags |= XDP_FLAGS_FRAGS_UNREADABLE;
>  }
>  
> +static __always_inline void xdp_buff_set_frag_pp(struct xdp_buff *xdp)
> +{
> +	xdp->flags |= XDP_FLAGS_FRAGS_PAGE_POOL;
> +}
> +
> +static __always_inline bool xdp_buff_is_frag_pp(const struct xdp_buff *xdp)
> +{
> +	return !!(xdp->flags & XDP_FLAGS_FRAGS_PAGE_POOL);
> +}
> +
>  static __always_inline u32 xdp_buff_get_skb_flags(const struct xdp_buff *xdp)
>  {
>  	return xdp->flags;
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 38336858c168..be36020484b6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5532,6 +5532,11 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
>  	if (skb_is_nonlinear(skb)) {
>  		skb_shinfo(skb)->xdp_frags_size = skb->data_len;
>  		xdp_buff_set_frags_flag(xdp);
> +		/* A nonlinear skb was cow'd into page_pool memory by
> +		 * skb_cow_data_for_xdp() before we got here, so the frags must
> +		 * be freed to that pool, not via the rxq's MEM_TYPE_PAGE_SHARED.
> +		 */
> +		xdp_buff_set_frag_pp(xdp);
>  	} else {
>  		xdp_buff_clear_frags_flag(xdp);
>  	}
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 61940e753552..d34ba56d79d8 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4378,6 +4378,13 @@ static bool bpf_xdp_shrink_data(struct xdp_buff *xdp, skb_frag_t *frag,
>  	if (mem_type == MEM_TYPE_XSK_BUFF_POOL) {
>  		netmem = 0;
>  		zc_frag = bpf_xdp_shrink_data_zc(xdp, shrink, tail, release);
> +	} else if (xdp_buff_is_frag_pp(xdp)) {
> +		/*
> +		 * Skb-backed XDP (generic XDP, veth) cow's the frags into a
> +		 * page_pool while the rxq stays MEM_TYPE_PAGE_SHARED, so free
> +		 * the frag to the pool, not via page_frag_free().
> +		 */
> +		mem_type = MEM_TYPE_PAGE_POOL;
>  	}
>  
>  	if (release) {
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH bpf v2 1/2] bpf, veth: xdp: fix page_pool page leak on skb-backed XDP

[Jiayuan Chen]

On 8/24/26 6:31 PM, Lorenzo Bianconi wrote:
>> bpf_xdp_shrink_data() frees a released frag via __xdp_return() using
>> xdp->rxq->mem.type, but that type is wrong for skb-backed XDP: the skb is
>> cow'd into page_pool memory while the rxq still says MEM_TYPE_PAGE_SHARED,
>> so the page_pool page is freed with page_frag_free() and we hit
>> "Bad page state ... page_pool leak".
>>
>> Both generic XDP and veth are affected. A non-linear skb is cow'd into
>> page_pool memory (skb_cow_data_for_xdp() -> skb_pp_cow_data() for generic
>> XDP, veth_convert_skb_to_xdp_buff() for veth), so its frags become
>> page_pool pages while the rxq keeps MEM_TYPE_PAGE_SHARED.
>>
>> We can't just fix rxq->mem.type in place:
>> - generic XDP: xdp->rxq is dev->_rx[queue].xdp_rxq (see
>>    bpf_prog_run_generic_xdp()), a shared rxq that other CPUs may access in
>>    parallel, so we must not write to it.
>> - veth: rq->xdp_rxq.mem is shared per-queue state that veth resets on XDP
>>    teardown, and with GRO that reset runs without stopping in-flight NAPI,
>>    so a type stashed there can be clobbered under a packet still in flight.
>>
>> Adding a check in __xdp_return() or bpf_xdp_shrink_data() itself is not an
>> option either: without recording it somewhere, both can only guess the
>> frag's memory type, which quickly gets confusing.
>>
>> So record it in the xdp_buff. Add a XDP_FLAGS_FRAGS_PAGE_POOL flag; the two
>> skb-cow sites set it, and bpf_xdp_shrink_data() frees the frag to the
>> page_pool when it is set, otherwise it keeps falling back to
>> xdp->rxq->mem.type unchanged. No other path changes behaviour.
>>
>> Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic mode")
>> Fixes: 0ebab78cbcbf ("net: veth: add page_pool for page recycling")
> Hi Jiayuan Chen,
>
> thx for fixing it. Can we do something like the patch below instead?
>
> Regards,
> Lorenzo
>
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 1d679e8fd649..4ed659b58141 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -433,16 +433,16 @@ EXPORT_SYMBOL_GPL(xdp_rxq_info_attach_page_pool);
>   void __xdp_return(netmem_ref netmem, enum xdp_mem_type mem_type,
>   		  bool napi_direct, struct xdp_buff *xdp)
>   {
> +	netmem_ref head_netmem = netmem_compound_head(netmem);
> +	if (netmem_is_pp(head_netmem))
> +		mem_type = MEM_TYPE_PAGE_POOL;
> +
>   	switch (mem_type) {
>   	case MEM_TYPE_PAGE_POOL:
> -		netmem = netmem_compound_head(netmem);
>   		if (napi_direct && xdp_return_frame_no_direct())
>   			napi_direct = false;
> -		/* No need to check netmem_is_pp() as mem->type knows this a
> -		 * page_pool page
> -		 */
> -		page_pool_put_full_netmem(netmem_get_pp(netmem), netmem,
> -					  napi_direct);
> +		page_pool_put_full_netmem(netmem_get_pp(head_netmem),
> +					  head_netmem, napi_direct);
>   		break;
>   	case MEM_TYPE_PAGE_SHARED:
>   		page_frag_free(__netmem_address(netmem));


Hi Lorenzo,

I tried this, but it regresses the bpf selftest with a page_pool ref 
underflow (0 warns on master, 45 with the patch):

     WARNING: include/net/page_pool/helpers.h:297 at 
page_pool_alloc_frag_netmem
     skb_pp_cow_data
     veth_xdp_rcv_skb


On XDP_TX/XDP_REDIRECT veth has to take plain page refs via get_page() 
(veth_xdp_get()) and then
consume_skb(): the skb itself must be freed while the data pages stay 
alive for the frame. consume_skb()
already returns the skb's page_pool ref, so what the frame holds 
afterwards is a plain page ref, to be
dropped with page_frag_free().

netmem_is_pp() can't see that: it only says the page still belongs to a 
pool (other users may still hold pool refs on the same page),
not what kind of ref we're dropping. So __xdp_return() turns those 
plain-ref drops into a second pool
put and pp_ref goes negative.

That's why I kept the type in the xdp_buff and only override it in the 
shrink path, where we know the
frag ref is the cow'd page_pool one.

Regards,
Jiayuan

Re: [PATCH bpf v2 1/2] bpf, veth: xdp: fix page_pool page leak on skb-backed XDP

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 4729 bytes --]

> 
> On 8/24/26 6:31 PM, Lorenzo Bianconi wrote:
> > > bpf_xdp_shrink_data() frees a released frag via __xdp_return() using
> > > xdp->rxq->mem.type, but that type is wrong for skb-backed XDP: the skb is
> > > cow'd into page_pool memory while the rxq still says MEM_TYPE_PAGE_SHARED,
> > > so the page_pool page is freed with page_frag_free() and we hit
> > > "Bad page state ... page_pool leak".
> > > 
> > > Both generic XDP and veth are affected. A non-linear skb is cow'd into
> > > page_pool memory (skb_cow_data_for_xdp() -> skb_pp_cow_data() for generic
> > > XDP, veth_convert_skb_to_xdp_buff() for veth), so its frags become
> > > page_pool pages while the rxq keeps MEM_TYPE_PAGE_SHARED.
> > > 
> > > We can't just fix rxq->mem.type in place:
> > > - generic XDP: xdp->rxq is dev->_rx[queue].xdp_rxq (see
> > >    bpf_prog_run_generic_xdp()), a shared rxq that other CPUs may access in
> > >    parallel, so we must not write to it.
> > > - veth: rq->xdp_rxq.mem is shared per-queue state that veth resets on XDP
> > >    teardown, and with GRO that reset runs without stopping in-flight NAPI,
> > >    so a type stashed there can be clobbered under a packet still in flight.
> > > 
> > > Adding a check in __xdp_return() or bpf_xdp_shrink_data() itself is not an
> > > option either: without recording it somewhere, both can only guess the
> > > frag's memory type, which quickly gets confusing.
> > > 
> > > So record it in the xdp_buff. Add a XDP_FLAGS_FRAGS_PAGE_POOL flag; the two
> > > skb-cow sites set it, and bpf_xdp_shrink_data() frees the frag to the
> > > page_pool when it is set, otherwise it keeps falling back to
> > > xdp->rxq->mem.type unchanged. No other path changes behaviour.
> > > 
> > > Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic mode")
> > > Fixes: 0ebab78cbcbf ("net: veth: add page_pool for page recycling")
> > Hi Jiayuan Chen,
> > 
> > thx for fixing it. Can we do something like the patch below instead?
> > 
> > Regards,
> > Lorenzo
> > 
> > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > index 1d679e8fd649..4ed659b58141 100644
> > --- a/net/core/xdp.c
> > +++ b/net/core/xdp.c
> > @@ -433,16 +433,16 @@ EXPORT_SYMBOL_GPL(xdp_rxq_info_attach_page_pool);
> >   void __xdp_return(netmem_ref netmem, enum xdp_mem_type mem_type,
> >   		  bool napi_direct, struct xdp_buff *xdp)
> >   {
> > +	netmem_ref head_netmem = netmem_compound_head(netmem);
> > +	if (netmem_is_pp(head_netmem))
> > +		mem_type = MEM_TYPE_PAGE_POOL;
> > +
> >   	switch (mem_type) {
> >   	case MEM_TYPE_PAGE_POOL:
> > -		netmem = netmem_compound_head(netmem);
> >   		if (napi_direct && xdp_return_frame_no_direct())
> >   			napi_direct = false;
> > -		/* No need to check netmem_is_pp() as mem->type knows this a
> > -		 * page_pool page
> > -		 */
> > -		page_pool_put_full_netmem(netmem_get_pp(netmem), netmem,
> > -					  napi_direct);
> > +		page_pool_put_full_netmem(netmem_get_pp(head_netmem),
> > +					  head_netmem, napi_direct);
> >   		break;
> >   	case MEM_TYPE_PAGE_SHARED:
> >   		page_frag_free(__netmem_address(netmem));
> 
> 
> Hi Lorenzo,
> 
> I tried this, but it regresses the bpf selftest with a page_pool ref
> underflow (0 warns on master, 45 with the patch):
> 
>     WARNING: include/net/page_pool/helpers.h:297 at
> page_pool_alloc_frag_netmem
>     skb_pp_cow_data
>     veth_xdp_rcv_skb
> 
> 
> On XDP_TX/XDP_REDIRECT veth has to take plain page refs via get_page()
> (veth_xdp_get()) and then
> consume_skb(): the skb itself must be freed while the data pages stay alive
> for the frame. consume_skb()
> already returns the skb's page_pool ref, so what the frame holds afterwards
> is a plain page ref, to be
> dropped with page_frag_free().
> 
> netmem_is_pp() can't see that: it only says the page still belongs to a pool
> (other users may still hold pool refs on the same page),
> not what kind of ref we're dropping. So __xdp_return() turns those plain-ref
> drops into a second pool
> put and pp_ref goes negative.
> 
> That's why I kept the type in the xdp_buff and only override it in the
> shrink path, where we know the
> frag ref is the cow'd page_pool one.
> 
> Regards,
> Jiayuan
> 

Right. I can see the point now :). IIUC we are currently able to trigger
the issue just on xdp fragments running bpf_xdp_shrink_data() but the problem
theoretically occurs even for the xdp->data, right? (it is rallocated using the
page_pool in skb_pp_cow_data()). Is it better to always set this new flag when
the buffers are reallocated via skb_pp_cow_data()? (Maybe renaming it in
something like XDP_FLAGS_DATA_FROM_PP).

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH bpf v2 1/2] bpf, veth: xdp: fix page_pool page leak on skb-backed XDP

[Jiayuan Chen]

On 8/24/26 10:50 PM, Lorenzo Bianconi wrote:
>> On 8/24/26 6:31 PM, Lorenzo Bianconi wrote:
>>>> bpf_xdp_shrink_data() frees a released frag via __xdp_return() using
>>>> xdp->rxq->mem.type, but that type is wrong for skb-backed XDP: the skb is
>>>> cow'd into page_pool memory while the rxq still says MEM_TYPE_PAGE_SHARED,
>>>> so the page_pool page is freed with page_frag_free() and we hit
>>>> "Bad page state ... page_pool leak".
>>>>
>>>> Both generic XDP and veth are affected. A non-linear skb is cow'd into
>>>> page_pool memory (skb_cow_data_for_xdp() -> skb_pp_cow_data() for generic
>>>> XDP, veth_convert_skb_to_xdp_buff() for veth), so its frags become
>>>> page_pool pages while the rxq keeps MEM_TYPE_PAGE_SHARED.
>>>>
>>>> We can't just fix rxq->mem.type in place:
>>>> - generic XDP: xdp->rxq is dev->_rx[queue].xdp_rxq (see
>>>>     bpf_prog_run_generic_xdp()), a shared rxq that other CPUs may access in
>>>>     parallel, so we must not write to it.
>>>> - veth: rq->xdp_rxq.mem is shared per-queue state that veth resets on XDP
>>>>     teardown, and with GRO that reset runs without stopping in-flight NAPI,
>>>>     so a type stashed there can be clobbered under a packet still in flight.
>>>>
>>>> Adding a check in __xdp_return() or bpf_xdp_shrink_data() itself is not an
>>>> option either: without recording it somewhere, both can only guess the
>>>> frag's memory type, which quickly gets confusing.
>>>>
>>>> So record it in the xdp_buff. Add a XDP_FLAGS_FRAGS_PAGE_POOL flag; the two
>>>> skb-cow sites set it, and bpf_xdp_shrink_data() frees the frag to the
>>>> page_pool when it is set, otherwise it keeps falling back to
>>>> xdp->rxq->mem.type unchanged. No other path changes behaviour.
>>>>
>>>> Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic mode")
>>>> Fixes: 0ebab78cbcbf ("net: veth: add page_pool for page recycling")
>>> Hi Jiayuan Chen,
>>>
>>> thx for fixing it. Can we do something like the patch below instead?
>>>
>>> Regards,
>>> Lorenzo
>>>
>>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>>> index 1d679e8fd649..4ed659b58141 100644
>>> --- a/net/core/xdp.c
>>> +++ b/net/core/xdp.c
>>> @@ -433,16 +433,16 @@ EXPORT_SYMBOL_GPL(xdp_rxq_info_attach_page_pool);
>>>    void __xdp_return(netmem_ref netmem, enum xdp_mem_type mem_type,
>>>    		  bool napi_direct, struct xdp_buff *xdp)
>>>    {
>>> +	netmem_ref head_netmem = netmem_compound_head(netmem);
>>> +	if (netmem_is_pp(head_netmem))
>>> +		mem_type = MEM_TYPE_PAGE_POOL;
>>> +
>>>    	switch (mem_type) {
>>>    	case MEM_TYPE_PAGE_POOL:
>>> -		netmem = netmem_compound_head(netmem);
>>>    		if (napi_direct && xdp_return_frame_no_direct())
>>>    			napi_direct = false;
>>> -		/* No need to check netmem_is_pp() as mem->type knows this a
>>> -		 * page_pool page
>>> -		 */
>>> -		page_pool_put_full_netmem(netmem_get_pp(netmem), netmem,
>>> -					  napi_direct);
>>> +		page_pool_put_full_netmem(netmem_get_pp(head_netmem),
>>> +					  head_netmem, napi_direct);
>>>    		break;
>>>    	case MEM_TYPE_PAGE_SHARED:
>>>    		page_frag_free(__netmem_address(netmem));
>>
>> Hi Lorenzo,
>>
>> I tried this, but it regresses the bpf selftest with a page_pool ref
>> underflow (0 warns on master, 45 with the patch):
>>
>>      WARNING: include/net/page_pool/helpers.h:297 at
>> page_pool_alloc_frag_netmem
>>      skb_pp_cow_data
>>      veth_xdp_rcv_skb
>>
>>
>> On XDP_TX/XDP_REDIRECT veth has to take plain page refs via get_page()
>> (veth_xdp_get()) and then
>> consume_skb(): the skb itself must be freed while the data pages stay alive
>> for the frame. consume_skb()
>> already returns the skb's page_pool ref, so what the frame holds afterwards
>> is a plain page ref, to be
>> dropped with page_frag_free().
>>
>> netmem_is_pp() can't see that: it only says the page still belongs to a pool
>> (other users may still hold pool refs on the same page),
>> not what kind of ref we're dropping. So __xdp_return() turns those plain-ref
>> drops into a second pool
>> put and pp_ref goes negative.
>>
>> That's why I kept the type in the xdp_buff and only override it in the
>> shrink path, where we know the
>> frag ref is the cow'd page_pool one.
>>
>> Regards,
>> Jiayuan
>>
> Right. I can see the point now :). IIUC we are currently able to trigger
> the issue just on xdp fragments running bpf_xdp_shrink_data() but the problem
> theoretically occurs even for the xdp->data, right? (it is rallocated using the
> page_pool in skb_pp_cow_data()). Is it better to always set this new flag when
> the buffers are reallocated via skb_pp_cow_data()? (Maybe renaming it in
> something like XDP_FLAGS_DATA_FROM_PP).
>
> Regards,
> Lorenzo


Hi Lorenzo,

Theoretically you are right, the linear part has the same issue.
But in practice the head is always freed via consume_skb()/kfree_skb(), 
which are pp-aware.
If we really want to go that way, we'd probably have to touch quite a 
few function signatures.
As a fix that also needs backporting, I think the current change is the 
minimal one that covers
all the reachable cases(happy to hear a better idea.).

Re: [PATCH bpf v2 1/2] bpf, veth: xdp: fix page_pool page leak on skb-backed XDP

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
bpf, veth: xdp: fix page_pool page leak on skb-backed XDP

This adds a new xdp_buff flag, XDP_FLAGS_FRAGS_PAGE_POOL, which the two
skb-cow sites (generic XDP and veth) set when a non-linear skb has been
cow'd into page_pool memory.  bpf_xdp_shrink_data() then frees a released
frag to the page_pool instead of using xdp->rxq->mem.type.

Closes: https://syzkaller.appspot.com/bug?extid=237bbeed8dfe0699b7f5

> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 6ed3ee81153fb..0afa0661ada16 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -775,6 +775,11 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
>  	if (skb_shinfo(skb)->nr_frags) {
>  		skb_shinfo(skb)->xdp_frags_size = skb->data_len;
>  		xdp_buff_set_frags_flag(xdp);
> +		/* A nonlinear skb was cow'd into rq->page_pool above, so the
> +		 * frags must be freed to that pool, not via the rxq's
> +		 * MEM_TYPE_PAGE_SHARED.
> +		 */
> +		xdp_buff_set_frag_pp(xdp);
>  	} else {
>  		xdp_buff_clear_frags_flag(xdp);
>  	}

[Severity: Critical]
Does this flag survive past the point where veth deliberately drops the
page_pool association?

In veth_xdp_rcv_skb(), the XDP_TX and XDP_REDIRECT arms do:

	case XDP_TX:
		veth_xdp_get(xdp);
		consume_skb(skb);
		xdp->rxq->mem = rq->xdp_mem;

veth_xdp_get() takes a plain get_page() on each frag, then consume_skb()
-> skb_release_data() -> napi_pp_put_page() finds page_ref_count() != 1,
so page_pool_return_netmem() -> page_pool_clear_pp_info() sets
netmem->pp = NULL.  veth then resets rxq->mem to MEM_TYPE_PAGE_SHARED,
which matches the now plain pages, but XDP_FLAGS_FRAGS_PAGE_POOL is never
cleared.

The flag then leaves the buff:

include/net/xdp.h:xdp_update_frame_from_buff() {
	...
	xdp_frame->flags = xdp->flags;
}

include/net/xdp.h:xdp_convert_frame_to_buff() {
	...
	xdp->flags = frame->flags;
}

so a second-stage consumer that rebuilds a buff from the frame and runs a
program inherits it.  veth_xdp_rcv_one() does exactly that, with no
xdp_init_buff() to reset flags:

drivers/net/veth.c:veth_xdp_rcv_one() {
	...
	xdp_convert_frame_to_buff(frame, xdp);
	xdp->rxq = &rq->xdp_rxq;
	vxbuf.skb = NULL;

	act = bpf_prog_run_xdp(xdp_prog, xdp);
}

cpu_map_bpf_prog_run_xdp() and dev_map_bpf_prog_run() inherit frame->flags
the same way.

If that peer program calls bpf_xdp_adjust_tail() with a negative offset
that consumes a whole frag, bpf_xdp_shrink_data() takes the new branch and
forces mem_type = MEM_TYPE_PAGE_POOL for a page that is no longer a
page_pool page.  __xdp_return() skips the netmem_is_pp() check for that
type:

net/core/xdp.c:__xdp_return() {
	...
		/* No need to check netmem_is_pp() as mem->type knows this a
		 * page_pool page
		 */
		page_pool_put_full_netmem(netmem_get_pp(netmem), netmem,
					  napi_direct);
}

netmem_get_pp() is NULL here, so page_pool_put_unrefed_netmem() reads
pool->cpuid in page_pool_napi_local() (or pool->dma_sync on PREEMPT_RT)
off a NULL pool in softirq context.  Since pp_ref_count is still 1 for a
frag that owns a whole page, is this deterministic rather than
refcount-dependent?  A veth pair with an xdp.frags program on both sides
looks sufficient to reach it.

Would clearing the flag in the XDP_TX / XDP_REDIRECT arms next to
xdp->rxq->mem = rq->xdp_mem, or masking it out in
xdp_update_frame_from_buff(), be the right fix?

Related to this, the commit message says:

    So record it in the xdp_buff. Add a XDP_FLAGS_FRAGS_PAGE_POOL flag; the two
    skb-cow sites set it, and bpf_xdp_shrink_data() frees the frag to the
    page_pool when it is set, otherwise it keeps falling back to
    xdp->rxq->mem.type unchanged. No other path changes behaviour.

Given that xdp_frame->flags carries the bit into frame-based paths, is "No
other path changes behaviour" accurate?

> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index aa742f413c358..b389dc527adce 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -81,6 +81,10 @@ enum xdp_buff_flags {
>  	 * XDP program is not attached.
>  	 */
>  	XDP_FLAGS_FRAGS_UNREADABLE	= BIT(2),
> +	/* frags are page_pool memory even though rxq->mem.type is not: a
> +	 * skb-backed XDP buff (generic XDP, veth) is cow'd into a page_pool.
> +	 */
> +	XDP_FLAGS_FRAGS_PAGE_POOL	= BIT(3),
>  };

[Severity: Medium]
This comment documents a general ownership rule for frags, but is the rule
only implemented in bpf_xdp_shrink_data()?

The other frag release paths still key exclusively on rxq->mem.type or
xdpf->mem_type and never consult xdp_buff_is_frag_pp():

net/core/xdp.c:xdp_return_frag() {
	__xdp_return(netmem, xdp->rxq->mem.type, true, NULL);
}

net/core/xdp.c:xdp_return_buff() {
	...
	for (u32 i = 0; i < sinfo->nr_frags; i++)
		__xdp_return(skb_frag_netmem(&sinfo->frags[i]),
			     xdp->rxq->mem.type, true, xdp);
}

plus the frame-side loops used by xdp_return_frame(),
xdp_return_frame_rx_napi() and xdp_return_frame_bulk(), which use
xdpf->mem_type.

xdp_convert_buff_to_frame() also stores two independent descriptions of the
same memory in one frame:

include/net/xdp.h:xdp_convert_buff_to_frame() {
	...
	if (unlikely(xdp_update_frame_from_buff(xdp, xdp_frame) < 0))
		return NULL;

	/* rxq only valid until napi_schedule ends, convert to xdp_mem_type */
	xdp_frame->mem_type = xdp->rxq->mem.type;
}

so xdp_frame->flags can say page_pool while xdp_frame->mem_type says
MEM_TYPE_PAGE_SHARED.  I could not construct a live caller that frees a
flagged buff through the generic helpers today, but should the contract be
made consistent (either honour the flag in the generic release helpers, or
scope the comment to the shrink path only) so a future caller does not
silently reintroduce the same "Bad page state ... page_pool leak"?

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 61940e7535523..d34ba56d79d8f 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4378,6 +4378,13 @@ static bool bpf_xdp_shrink_data(struct xdp_buff *xdp, skb_frag_t *frag,
>  	if (mem_type == MEM_TYPE_XSK_BUFF_POOL) {
>  		netmem = 0;
>  		zc_frag = bpf_xdp_shrink_data_zc(xdp, shrink, tail, release);
> +	} else if (xdp_buff_is_frag_pp(xdp)) {
> +		/*
> +		 * Skb-backed XDP (generic XDP, veth) cow's the frags into a
> +		 * page_pool while the rxq stays MEM_TYPE_PAGE_SHARED, so free
> +		 * the frag to the pool, not via page_frag_free().
> +		 */
> +		mem_type = MEM_TYPE_PAGE_POOL;
>  	}
>  
>  	if (release) {

[ ... ]

Re: [PATCH bpf v2 2/2] selftests/bpf: add xdp_shrink_frags

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
selftests/bpf: add xdp_shrink_frags

Adds a new test_progs test with two subtests (tun and veth) that attach an
xdp.frags program shrinking the tail by 3000 bytes so a whole frag is
released.  The intent is to reproduce a page_pool frag being freed with the
wrong memory type on skb-backed XDP paths.

Two questions below about what the test actually observes.

> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c
> new file mode 100644
> index 0000000000000..f3d8a84a6dc9c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_shrink_frags.c
> @@ -0,0 +1,163 @@

[ ... ]

> +/*
> + * Similar to flow_dissector.c: writev() an IFF_NAPI_FRAGS tap to build a
> + * nonlinear skb (sized for 4K pages, like xdp_adjust_tail.c) that tun runs
> + * through do_xdp_generic().
> + */
> +static void test_tun(struct xdp_shrink_frags *skel)
> +{
> +	__u8 head[74], frag1[2048], frag2[2048];

[Severity: Medium]
Is the 74 + 2048 + 2048 byte frame guaranteed to still be nonlinear when the
program runs on kernels with PAGE_SIZE larger than 4096?

skb_pp_cow_data() in net/core/skbuff.c bounds the linear head by the page
size:

	max_head_size = SKB_WITH_OVERHEAD(PAGE_SIZE - headroom);
	...
	size = min_t(u32, skb->len, max_head_size);
	...
	for (i = 0; i < MAX_SKB_FRAGS && off < skb->len; i++) {

With PAGE_SIZE 16384 or 65536 the whole 4170-byte frame fits in the head, so
the frag loop body never runs and the xdp_buff has no frags.

tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c handles this by
branching on getpagesize():

	int page_size = getpagesize();
	...
		if (page_size == 65536)
			test_xdp_adjust_frags_tail_grow_64k();

Would a getpagesize() branch or skip be appropriate here too, given that
config.aarch64 is shipped in the bpf selftests?

[ ... ]

> +	n = writev(tap_fd, iov, ARRAY_SIZE(iov));
> +	ASSERT_EQ(n, sizeof(head) + sizeof(frag1) + sizeof(frag2), "writev");
> +
> +	usleep(100 * 1000);
> +	ASSERT_GT(skel->bss->shrink_ran, 0, "xdp_prog_ran");

[Severity: Medium]
Can this subtest distinguish a buggy kernel from a fixed one?

The failure mode the commit message names ("Bad page state ... page_pool
leak") is produced by bad_page() in mm/page_alloc.c, with the reason string
set in page_bad_reason():

	if (unlikely(page_pool_page_is_pp(page)))
		bad_reason = "page_pool leak";

bad_page() is a static void that only emits pr_alert()/dump_page()/
dump_stack() and then:

	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);

No error is propagated back to bpf_xdp_adjust_tail(), writev() or ping, and
the test does not read /proc/sys/kernel/tainted, scan kmsg, or look at
page_pool inflight statistics.

So the assertions here (bpf_xdp_attach, the writev length, and shrink_ran)
all succeed identically on a kernel with and without the fix.  Would it make
sense to add an observable oracle (taint bit, kmsg scan, or page_pool
stats), or alternatively to state in the commit message that this is a splat
generator requiring manual dmesg inspection?

[ ... ]

> +static void test_veth(struct xdp_shrink_frags *skel)
> +{

[ ... ]

> +	SYS_NOFAIL("ip netns exec " VETH_NETNS
> +		   " ping -q -s 5000 -c 3 -W 1 " VETH_LOCAL_IP);
> +
> +	ASSERT_GT(skel->bss->shrink_ran, 0, "xdp_prog_ran");

[Severity: Medium]
The same two concerns apply here.  The ping runs under SYS_NOFAIL and the
only assertion is shrink_ran > 0, so nothing observes whether the released
frag went back to its page_pool.

Additionally, the frag layout of a ~5 KB ping through an 8000-MTU veth is
decided by the sender's SG/GRO behaviour rather than by the test.  If the
skb ends up with a single frag larger than 3000 bytes, the shrink happens in
place and no page is released at all.

On large-PAGE_SIZE kernels veth_convert_skb_to_xdp_buff() takes the other
branch after the COW leaves no frags:

	if (skb_shinfo(skb)->nr_frags) {
		...
		xdp_buff_set_frag_pp(xdp);
	} else {
		xdp_buff_clear_frags_flag(xdp);
	}

which bypasses the frag-release code entirely.

> diff --git a/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c b/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c
> new file mode 100644
> index 0000000000000..62e3791ac81d9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/xdp_shrink_frags.c
> @@ -0,0 +1,23 @@

[ ... ]

> +	/*
> +	 * The program is loaded with BPF_F_XDP_HAS_FRAGS (xdp.frags), so a
> +	 * nonlinear skb entering generic XDP is cow'd into page_pool memory
> +	 * before we run. Shrinking the tail far enough releases at least one
> +	 * whole frag, which must be returned to its page_pool. Count only a
> +	 * successful shrink so a too-small frame (e.g. ARP) does not satisfy
> +	 * the test.
> +	 */
> +	if (bpf_xdp_adjust_tail(ctx, -3000) == 0)
> +		__sync_fetch_and_add(&shrink_ran, 1);

[Severity: Medium]
Does a return value of 0 here really imply a frag was released?

bpf_xdp_adjust_tail() in net/core/filter.c only enters the frag path when
the buffer is nonlinear:

	if (unlikely(xdp_buff_has_frags(xdp))) { /* non-linear xdp buff */
		if (offset < 0)
			return bpf_xdp_frags_shrink_tail(xdp, -offset);
	...
	xdp->data_end = data_end;

	return 0;

A purely linear buffer takes the bottom path and also returns 0, so
shrink_ran gets incremented without bpf_xdp_shrink_data() ever running.
That seems to contradict the comment's claim that only a genuine
frag-releasing shrink is counted.

Would asserting somewhere that the buffer actually had frags (for example
comparing ctx->data_end - ctx->data against the total frame length) make the
counter mean what the comment says?

Re: [PATCH bpf v2 1/2] bpf, veth: xdp: fix page_pool page leak on skb-backed XDP

[Jakub Kicinski]

On Mon, 24 Aug 2026 11:06:43 +0800 Jiayuan Chen wrote:
> bpf_xdp_shrink_data() frees a released frag via __xdp_return() using
> xdp->rxq->mem.type, but that type is wrong for skb-backed XDP: the skb is
> cow'd into page_pool memory while the rxq still says MEM_TYPE_PAGE_SHARED,
> so the page_pool page is freed with page_frag_free() and we hit
> "Bad page state ... page_pool leak".

FWIW I like the idea if it can be made to work fully correctly.