* [PATCH ipsec v3] xfrm: iptfs: fix pp_ref_count underflow when sharing page_pool frags
@ 2026-09-24 19:59 Antony Antony
2026-09-30 6:10 ` Steffen Klassert
0 siblings, 1 reply; 3+ messages in thread
From: Antony Antony @ 2026-09-24 19:59 UTC (permalink / raw)
To: Christian Hopps, Steffen Klassert, Herbert Xu, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman
Cc: netdev, Antony Antony
skb frags are either page_pool pages tracked via pp_ref_count
(released by napi_pp_put_page()), or regular pages tracked via
_refcount (released by put_netmem()).
skb->pp_recycle was unbalanced and caused the underflow that
hit BUG().
Fix by taking the page_pool reference only when the destination has
pp_recycle set and the fragment's page is actually page_pool owned,
matching skb_pp_frag_ref(); fall back to a plain reference otherwise.
See the kernel splat, before. Observed under normal traffic when a page_pool
frag is shared into two extra skbs
[ 52.644633] ------------[ cut here ]------------
[ 52.644649] WARNING: ./include/net/page_pool/helpers.h:297 at page_pool_put_netmem.constprop.0+0x1f/0x40, CPU#0: swapper/0/0
[ 52.644665] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 7.3.0-rc2-00485-gcda8dccdef8d #19 PREEMPT(full)
[ 52.644669] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 52.644672] RIP: 0010:page_pool_put_netmem.constprop.0+0x1f/0x40
[ 52.644676] Code: 90 90 90 90 90 90 90 90 90 90 90 48 89 f0 48 83 e0 fe 48 8b 48 28 48 ff c9 74 20 48 83 c9 ff f0 48 0f c1 48 28 48 ff c9 79 07 <0f> 0b c3 cc cc cc cc 75 13 48 c7 40 28 01 00 00 00 0f b6 ca 83 ca
[ 52.644679] RSP: 0018:ffffc90000003c98 EFLAGS: 00010296
[ 52.644683] RAX: ffffea00041cc200 RBX: ffff888105335100 RCX: ffffffffffffffff
[ 52.644686] RDX: 0000000000000001 RSI: ffffea00041cc200 RDI: ffff8881013fa000
[ 52.644688] RBP: 000000000000000c R08: 000000000000005e R09: 0000000000000a00
[ 52.644690] R10: ffff888106834fc0 R11: 0000000000000020 R12: 0000000000000000
[ 52.644692] R13: ffff8881013fb000 R14: ffffea00041cc200 R15: ffff88810097d9c0
[ 52.644698] FS: 0000000000000000(0000) GS:ffff8881f887c000(0000) knlGS:0000000000000000
[ 52.644701] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 52.644703] CR2: 00007fd8753ec180 CR3: 0000000106f2e004 CR4: 0000000000170eb0
[ 52.644706] Call Trace:
[ 52.644710] <IRQ>
[ 52.644712] page_to_skb+0x1f3/0x210
[ 52.644719] receive_buf+0x712/0xca0
[ 52.644724] ? detach_buf_split_in_order+0x5d/0x110
[ 52.644730] virtnet_poll+0x1da/0x460
[ 52.644736] __napi_poll.constprop.0+0x2a/0x120
[ 52.644745] net_rx_action+0x11a/0x230
[ 52.644748] ? raise_softirq_irqoff+0x5/0x20
[ 52.644754] ? __napi_schedule+0x31/0x50
[ 52.644759] ? vring_interrupt+0x77/0x90
[ 52.644765] handle_softirqs+0x11e/0x270
[ 52.644769] __irq_exit_rcu+0x53/0xf0
[ 52.644773] common_interrupt+0x95/0xc0
[ 52.644782] </IRQ>
[ 52.644784] <TASK>
[ 52.644786] asm_common_interrupt+0x22/0x40
[ 52.644790] RIP: 0010:default_idle+0xb/0x20
[ 52.644794] Code: 00 4d 29 c8 4c 01 c7 4c 29 c2 e9 6e ff ff ff 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 eb 07 0f 00 2d 1d c6 01 00 fb f4 <fa> c3 cc cc cc cc 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 90
[ 52.644797] RSP: 0018:ffffffff82a03e08 EFLAGS: 00000212
[ 52.644800] RAX: 0000000000000000 RBX: ffffffff82a0b480 RCX: 00000000ffff0e38
[ 52.644802] RDX: 0000000000000000 RSI: ffffffff82211089 RDI: 00000000000ec84c
[ 52.644804] RBP: 0000000000000000 R08: 0000000000000002 R09: 0000000000000000
[ 52.644806] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 52.644808] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000013ab0
[ 52.644814] default_idle_call+0x3c/0x70
[ 52.644818] do_idle+0xdc/0x200
[ 52.644823] cpu_startup_entry+0x29/0x30
[ 52.644828] rest_init+0xe8/0xf0
[ 52.644833] ? __pfx_kernel_init+0x10/0x10
[ 52.644836] start_kernel+0x5fd/0x600
[ 52.644846] x86_64_start_reservations+0x20/0x20
[ 52.644850] x86_64_start_kernel+0xc9/0xd0
[ 52.644854] common_startup_64+0x129/0x148
[ 52.644860] </TASK>
[ 52.644862] ---[ end trace 0000000000000000 ]---
Fixes: 5f2b6a909574 ("xfrm: iptfs: add skb-fragment sharing code")
Fixes: b96ba312e21c ("xfrm: iptfs: share page fragments of inner packets")
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
v2->v3: check page is pp before pp_ref_count bump. create helper func
- Link to v2: https://patchwork.kernel.org/project/netdevbpf/patch/xfrm-iptfs-pp_ref_count-underflow-v1-1-5fb363833d41@secunet.com/
v1->v2: rebase to latest ipsec
- Link to v1: https://lore.kernel.org/all/xfrm-iptfs-pp_ref_count-underflow-v1-1-47b319c6d2f6@secunet.com/
---
net/xfrm/xfrm_iptfs.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c
index 6920940a35b4..229f84ac6a31 100644
--- a/net/xfrm/xfrm_iptfs.c
+++ b/net/xfrm/xfrm_iptfs.c
@@ -14,6 +14,7 @@
#include <net/icmp.h>
#include <net/ip6_route.h>
#include <net/inet_ecn.h>
+#include <net/page_pool/helpers.h>
#include <net/xfrm.h>
#include <crypto/aead.h>
@@ -449,6 +450,20 @@ static bool iptfs_skb_can_add_frags(const struct sk_buff *skb,
return true;
}
+static void iptfs_frag_ref(skb_frag_t *frag, bool recycle)
+{
+ struct page *head;
+
+ if (recycle && !skb_frag_is_net_iov(frag)) {
+ head = compound_head(skb_frag_page(frag));
+ if (page_pool_page_is_pp(head)) {
+ page_pool_ref_page(head);
+ return;
+ }
+ }
+ __skb_frag_ref(frag);
+}
+
/**
* iptfs_skb_add_frags() - add a range of fragment references into an skb
* @skb: skb to add references into
@@ -486,7 +501,7 @@ static int iptfs_skb_add_frags(struct sk_buff *skb,
tofrag->len -= offset;
offset = 0;
}
- __skb_frag_ref(tofrag);
+ iptfs_frag_ref(tofrag, skb->pp_recycle);
shinfo->nr_frags++;
shinfo->flags |= SKBFL_SHARED_FRAG;
@@ -2171,7 +2186,8 @@ static void iptfs_consume_frags(struct sk_buff *to, struct sk_buff *from)
new_truesize = SKB_TRUESIZE(skb_end_offset(from));
} else {
iptfs_skb_head_to_frag(from, &toi->frags[toi->nr_frags]);
- skb_frag_ref(to, toi->nr_frags++);
+ iptfs_frag_ref(&toi->frags[toi->nr_frags], to->pp_recycle);
+ toi->nr_frags++;
new_truesize = SKB_DATA_ALIGN(sizeof(struct sk_buff));
}
---
base-commit: 868f63c8bfafa9b827168c9126f85264c39c02ec
change-id: xfrm-iptfs-pp_ref_count-underflow-063ee0302600
Best regards,
--
Antony Antony <antony.antony@secunet.com>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH ipsec v3] xfrm: iptfs: fix pp_ref_count underflow when sharing page_pool frags
2026-09-24 19:59 [PATCH ipsec v3] xfrm: iptfs: fix pp_ref_count underflow when sharing page_pool frags Antony Antony
@ 2026-09-30 6:10 ` Steffen Klassert
2026-10-01 9:14 ` Antony Antony
0 siblings, 1 reply; 3+ messages in thread
From: Steffen Klassert @ 2026-09-30 6:10 UTC (permalink / raw)
To: Antony Antony
Cc: Christian Hopps, Herbert Xu, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, netdev
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
commit a719ecd0c8cfff38a8b6302ad3d654603fe9e7d1
Author: Antony Antony <antony.antony@secunet.com>
xfrm: iptfs: fix pp_ref_count underflow when sharing page_pool frags
This commit modifies xfrm iptfs packet processing to correctly track page_pool
references for shared fragments. It introduces iptfs_frag_ref() to selectively
bump the page_pool reference when the destination has pp_recycle set and the
fragment is owned by page_pool.
Link: https://patchwork.kernel.org/project/netdevbpf/patch/xfrm-iptfs-pp_ref_count-underflow-v1-1-5fb363833d41@secunet.com/
Link: https://lore.kernel.org/all/xfrm-iptfs-pp_ref_count-underflow-v1-1-47b319c6d2f6@secunet.com/
> diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c
> index 6920940a35b49..229f84ac6a31b 100644
> --- a/net/xfrm/xfrm_iptfs.c
> +++ b/net/xfrm/xfrm_iptfs.c
[ ... ]
> @@ -449,6 +450,20 @@ static bool iptfs_skb_can_add_frags(const struct sk_buff *skb,
> return true;
> }
>
> +static void iptfs_frag_ref(skb_frag_t *frag, bool recycle)
> +{
> + struct page *head;
> +
> + if (recycle && !skb_frag_is_net_iov(frag)) {
> + head = compound_head(skb_frag_page(frag));
> + if (page_pool_page_is_pp(head)) {
> + page_pool_ref_page(head);
> + return;
> + }
> + }
> + __skb_frag_ref(frag);
> +}
Does skipping net_iov fragments here cause a reference counting mismatch?
If a devmem net_iov fragment is processed by iptfs_frag_ref(), the check
!skb_frag_is_net_iov(frag) evaluates to false. This bypasses the page_pool
reference tracking and forces the fragment into the fallback path, which
increments the dmabuf_binding refcount via __skb_frag_ref().
However, when the destination SKB (with pp_recycle=true) is later freed,
skb_frag_unref() calls napi_pp_put_page(). Since netmem_is_pp() evaluates
to true for devmem fragments, this erroneously decrements the pp_ref_count
instead of releasing the binding refcount.
Could this lead to a pp_ref_count underflow and use-after-free of devmem
buffers during IPsec packet processing?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH ipsec v3] xfrm: iptfs: fix pp_ref_count underflow when sharing page_pool frags
2026-09-30 6:10 ` Steffen Klassert
@ 2026-10-01 9:14 ` Antony Antony
0 siblings, 0 replies; 3+ messages in thread
From: Antony Antony @ 2026-10-01 9:14 UTC (permalink / raw)
To: Steffen Klassert
Cc: Antony Antony, Christian Hopps, Herbert Xu, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, netdev
On Wed, Sep 30, 2026 at 08:10:11AM +0200, Steffen Klassert wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> commit a719ecd0c8cfff38a8b6302ad3d654603fe9e7d1
> Author: Antony Antony <antony.antony@secunet.com>
>
> xfrm: iptfs: fix pp_ref_count underflow when sharing page_pool frags
>
> This commit modifies xfrm iptfs packet processing to correctly track page_pool
> references for shared fragments. It introduces iptfs_frag_ref() to selectively
> bump the page_pool reference when the destination has pp_recycle set and the
> fragment is owned by page_pool.
>
> Link: https://patchwork.kernel.org/project/netdevbpf/patch/xfrm-iptfs-pp_ref_count-underflow-v1-1-5fb363833d41@secunet.com/
> Link: https://lore.kernel.org/all/xfrm-iptfs-pp_ref_count-underflow-v1-1-47b319c6d2f6@secunet.com/
>
> > diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c
> > index 6920940a35b49..229f84ac6a31b 100644
> > --- a/net/xfrm/xfrm_iptfs.c
> > +++ b/net/xfrm/xfrm_iptfs.c
> [ ... ]
> > @@ -449,6 +450,20 @@ static bool iptfs_skb_can_add_frags(const struct sk_buff *skb,
> > return true;
> > }
> >
> > +static void iptfs_frag_ref(skb_frag_t *frag, bool recycle)
> > +{
> > + struct page *head;
> > +
> > + if (recycle && !skb_frag_is_net_iov(frag)) {
> > + head = compound_head(skb_frag_page(frag));
> > + if (page_pool_page_is_pp(head)) {
> > + page_pool_ref_page(head);
> > + return;
> > + }
> > + }
> > + __skb_frag_ref(frag);
> > +}
>
> Does skipping net_iov fragments here cause a reference counting mismatch?
>
> If a devmem net_iov fragment is processed by iptfs_frag_ref(), the check
> !skb_frag_is_net_iov(frag) evaluates to false. This bypasses the page_pool
> reference tracking and forces the fragment into the fallback path, which
> increments the dmabuf_binding refcount via __skb_frag_ref().
>
> However, when the destination SKB (with pp_recycle=true) is later freed,
> skb_frag_unref() calls napi_pp_put_page(). Since netmem_is_pp() evaluates
> to true for devmem fragments, this erroneously decrements the pp_ref_count
> instead of releasing the binding refcount.
>
> Could this lead to a pp_ref_count underflow and use-after-free of devmem
> buffers during IPsec packet processing?
Possibly. I could not reproduce the devmem/netmem path and have no way
to test it here, which is why I left it out of this patch rather than
guess at a fix.
The issue this patch addresses is hit with a virtio NIC when traffic
running on two different physical hosts over network; I reproduced it and tested
the fix in that setup.
I see two options: apply v3 as is, or
I send a v4 that also handles devmem by following which I can only
compile-test. I'm fine with either; please let me know which you
prefer. Are there any devmem experts who could reveiew or test it?
I guess the following could work for devmem
if (skb_frag_netmem(frag)) {
page_pool_ref_netmem(netmem);
return;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-01 9:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24 19:59 [PATCH ipsec v3] xfrm: iptfs: fix pp_ref_count underflow when sharing page_pool frags Antony Antony
2026-09-30 6:10 ` Steffen Klassert
2026-10-01 9:14 ` Antony Antony
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox