Re: [PATCH ipsec v3] xfrm: iptfs: fix pp_ref_count underflow when sharing page_pool frags

[Antony Antony <antony@phenome.org>]

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;
}