Re: [PATCH net-next v1 2/5] net: skbuff: refcount spliced page_pool frags via skb_pipe_buf_ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: skbuff: missing refcount overflow check in skb_pipe_buf_get()
--

commit 9c79ad1ae20fce07ba4bd771a989ba25467dc207
Author: Mina Almasry <almasrymina@google.com>

net: skbuff: refcount spliced page_pool frags via skb_pipe_buf_ops

This commit updates skb_splice_bits() and skb_append_pagefrags() to acquire
and release fragment references via netmem_inc_ref(), netmem_dec_ref(), and
skb_frag_ref() using a new skb_pipe_buf_ops table.

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 48f6764117243..32a9dcdf2cb22 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3114,9 +3114,27 @@ EXPORT_SYMBOL(skb_copy_bits);
>   */
>  static void sock_spd_release(struct splice_pipe_desc *spd, unsigned int i)
>  {
> -	put_page(spd->pages[i]);
> +	netmem_dec_ref(page_to_netmem(spd->pages[i]));
>  }
>  
> +static void skb_pipe_buf_release(struct pipe_inode_info *pipe,
> +				 struct pipe_buffer *buf)
> +{
> +	netmem_dec_ref(page_to_netmem(buf->page));
> +}
> +
> +static bool skb_pipe_buf_get(struct pipe_inode_info *pipe,
> +			     struct pipe_buffer *buf)
> +{
> +	netmem_inc_ref(page_to_netmem(buf->page));
> +	return true;
> +}

[Severity: High]
Does replacing nosteal_pipe_buf_ops with skb_pipe_buf_ops in skb_splice_bits()
drop the page reference count overflow check when pipe buffers are duplicated?

Previously, nosteal_pipe_buf_ops used generic_pipe_buf_get(), which calls
try_get_page() and fails when the signed 32-bit page reference count is <= 0:

fs/pipe.c:generic_pipe_buf_get() {
	return try_get_page(buf->page);
}

include/linux/mm.h:try_get_page() {
	page = compound_head(page);
	if (WARN_ON_ONCE(page_ref_count(page) <= 0))
		return false;
	page_ref_inc(page);
	return true;
}

With skb_pipe_buf_get(), duplicating a spliced pipe buffer via tee() ->
link_pipe(), splice() -> splice_pipe_to_pipe(), or fuse_dev_splice_write()
invokes skb_pipe_buf_get() -> netmem_inc_ref() -> get_page() and
unconditionally returns true:

include/net/netmem.h:netmem_inc_ref() {
	if (netmem_is_net_iov(netmem))
		net_iov_inc_ref(netmem_to_net_iov(netmem));
	else
		get_page(netmem_to_page(netmem));
}

For any struct page-backed netmem using standard page refcounting (such as
sk_page_frag() pages from linear_to_page(), non-page_pool RX fragments, user
pages sent over a local socket, or page_pool pages whose pp_ref_count is 0, as
well as all struct pages at this commit prior to commit "net: remove
skb->pp_recycle and consolidate netmem refcounting"), can repeated pipe buffer
duplications push the page reference count past INT_MAX, triggering
VM_BUG_ON_FOLIO(folio_ref_zero_or_close_to_overflow(folio), folio) under
CONFIG_DEBUG_VM or wrapping _refcount to 0 and causing a use-after-free when
CONFIG_DEBUG_VM is disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103211424.127627-1-almasrymina@google.com?part=2