Netdev List
 help / color / mirror / Atom feed
* [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