Re: [PATCH net-next v2 1/2] net: netmem: document netmem and memory provider design in comments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: skb_try_coalesce() mixes page and net_iov frags, deadlocking tcp_recvmsg()
--

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

net: netmem: document netmem and memory provider design in comments

Clarify the netmem, memory provider, page_pool, and skb fragment design
principles in header and code comments. It documents that all fragments in
an skb must share the same backing memory type and adds a check to avoid
coalescing skbs with different fragment memory types.

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 43ebe61c7fc48..2c42a218dcc00 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -6206,6 +6206,9 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
>  	if (to->pp_recycle != from->pp_recycle)
>  		return false;
>  
> +	/* All frags in an skb must have the same backing netmem memory type;
> +	 * do not coalesce skbs with different frag memory types.
> +	 */
>  	if (skb_frags_readable(from) != skb_frags_readable(to))
>  		return false;
>  

[Severity: High]
Does this fully prevent mixing fragment types if the source skb has
linear payload?

If both skbs are unreadable, they pass this check. But if the source skb
has linear payload, skb_try_coalesce() unconditionally converts the
linear payload into a struct page fragment and appends it to the target skb:

skb_try_coalesce() {
    ...
    if (skb_headlen(from) != 0) {
        ...
        page = virt_to_head_page(from->head);
        ...
        skb_fill_page_desc(to, to_shinfo->nr_frags,
                           page, offset, skb_headlen(from));
        ...
    }
}

Would this result in the unreadable target skb containing a mix of net_iov
fragments and a struct page fragment?

If this mixed skb reaches tcp_recvmsg_locked(), could it cause a permanent
socket deadlock? An skb marked unreadable forces reads through
tcp_recvmsg_dmabuf(), which aborts upon encountering a readable struct page
fragment. This causes recvmsg to repeatedly return -EFAULT without advancing
the offset.

Additionally, on transmit paths, validate_xmit_unreadable_skb() only checks
the first fragment. Could mixed skbs bypass these transmit checks and reach
drivers, potentially causing issues if they incorrectly cast the page to
net_iov?

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