Re: [PATCH net-next 1/9] net: ftmac100: check for failure when pulling in the RX header

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: ftmac100: UAF and infinite NAPI loop on rx page allocation failure
--

commit 64e4a607347eed7af02ba45920da8e098d9fbc07
Author: Josef Bacik <josef@toxicpanda.com>

net: ftmac100: check for failure when pulling in the RX header

This patch updates ftmac100_rx_packet() to check for failures when pulling
the Ethernet header or linearizing small frames. It drops the frame if
either pskb_may_pull() or __skb_linearize() fails to prevent panics in
__skb_pull().

> diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
> index 40ba001d4b3fc..a1eb04ead2d8a 100644
> --- a/drivers/net/ethernet/faraday/ftmac100.c
> +++ b/drivers/net/ethernet/faraday/ftmac100.c
> @@ -469,15 +469,21 @@ static bool ftmac100_rx_packet(struct ftmac100 *priv, int *processed)
>  	if (length > 128) {
>  		skb->truesize += PAGE_SIZE;
>  		/* We pull the minimum amount into linear part */
> -		__pskb_pull_tail(skb, ETH_HLEN);
> +		ret = pskb_may_pull(skb, ETH_HLEN);
>  	} else {
>  		/* Small frames are copied into linear part to free one page */
> -		__pskb_pull_tail(skb, length);
> +		ret = !__skb_linearize(skb);
>  	}
>  	ftmac100_alloc_rx_page(priv, rxdes, GFP_ATOMIC);

[Severity: High]
What happens if ftmac100_alloc_rx_page() fails due to memory pressure?

If alloc_page() fails inside ftmac100_alloc_rx_page(), the function returns
early without calling ftmac100_rxdes_set_dma_own(). Because the error
is ignored here, the descriptor is left owned by the CPU.

When the NAPI loop wraps around, wouldn't ftmac100_rx_locate_first_segment()
return this exact same descriptor, leading to a second dma_unmap_page() and
attaching the already-assigned page to a new skb?

>  
>  	ftmac100_rx_pointer_advance(priv);
>  
> +	if (unlikely(!ret)) {
> +		netdev->stats.rx_dropped++;
> +		kfree_skb(skb);
> +		return true;
> +	}

[Severity: High]
Could this early return cause an infinite NAPI polling loop?

If __skb_linearize() fails, this block returns true but skips the
(*processed)++ increment at the end of the function.

If ftmac100_alloc_rx_page() also failed, leaving the descriptor CPU-owned,
wouldn't the NAPI poll loop repeatedly process this same descriptor without
advancing the processed count, completely locking up the CPU?

> +
>  	skb->protocol = eth_type_trans(skb, netdev);
>  
>  	netdev->stats.rx_packets++;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-b4-pskb-pull-tail-drivers-v1-0-9512b0fb977b@toxicpanda.com?part=1