Re: [PATCH net] net: mana: report RX checksum offload for IPv6

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] net: mana: memory leak on XDP drop path in mana_rx_skb() [1]
--

commit 44321b6481b9b9f9a70166beb9fc63a817e238f6
Author: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>

net: mana: report RX checksum offload for IPv6

This commit removes the rx_iphdr_csum_succeed check in mana_rx_skb() so
that IPv6 TCP and UDP packets with validated L4 checksums receive
CHECKSUM_UNNECESSARY.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index e6d494df22e4d..05bd4b4b8349e 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -2179,10 +2179,9 @@ static void mana_rx_skb(void *buf_va, bool from_pool,
>  	skb_checksum_none_assert(skb);
>  	skb_record_rx_queue(skb, rxq_idx);
>  
> -	if ((ndev->features & NETIF_F_RXCSUM) && cqe->rx_iphdr_csum_succeed) {
> -		if (cqe->rx_tcp_csum_succeed || cqe->rx_udp_csum_succeed)
> -			skb->ip_summed = CHECKSUM_UNNECESSARY;
> -	}
> +	if ((ndev->features & NETIF_F_RXCSUM) &&
> +	    (cqe->rx_tcp_csum_succeed || cqe->rx_udp_csum_succeed))
> +		skb->ip_summed = CHECKSUM_UNNECESSARY;

[Severity: High]
This problem wasn't introduced by this patch, but can mana_rx_skb() leak
rxq->xdp_save_va [1] on its drop path when rxq->frag_count > 1?

When a pre-allocated buffer (from_pool == false) is dropped, mana_rx_skb()
stores buf_va in rxq->xdp_save_va for reuse:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_rx_skb() {
	...
drop:
	if (from_pool) {
		...
	} else {
		WARN_ON_ONCE(rxq->xdp_save_va);
		/* Save for reuse */
		rxq->xdp_save_va = buf_va;
	}
}

However, mana_get_rxfrag() only checks and reclaims rxq->xdp_save_va when
rxq->frag_count == 1:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_get_rxfrag() {
	...
	if (rxq->frag_count == 1) {
		/* Reuse XDP dropped page if available */
		if (rxq->xdp_save_va) {
			va = rxq->xdp_save_va;
			page = virt_to_head_page(va);
			rxq->xdp_save_va = NULL;
		...
		return va;
	}

	page =  page_pool_dev_alloc_frag(rxq->page_pool, &offset,
					 rxq->alloc_size);
}

When rxq->frag_count > 1, mana_get_rxfrag() ignores rxq->xdp_save_va and
allocates a new fragment from the page pool, so a subsequent drop of a
pre-allocated buffer triggers the WARN_ON_ONCE() and overwrites
rxq->xdp_save_va, leaking the saved page.

[1] https://sashiko.dev/#/bug/linux-2ba39e78-7dc4-4027-a76e-f2d315147bad

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109142856.779955-1-hamzamahfooz@linux.microsoft.com?part=1