Re: [PATCH net-next 3/9] net: niu: 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:
- [Medium] net: torn read on 64-bit statistic rp->rx_dropped without u64_stats_sync
--

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

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

This patch updates niu_process_rx_pkt() to use pskb_may_pull() to ensure
there is enough room in the SKB head for the hardware RX and Ethernet
headers. If the pull fails, the packet is now dropped instead of relying
on the buffer's initial state.

> diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
> index c74a97fe54641..d1c0e868004d1 100644
> --- a/drivers/net/ethernet/sun/niu.c
> +++ b/drivers/net/ethernet/sun/niu.c
> @@ -3488,7 +3488,11 @@ static int niu_process_rx_pkt(struct napi_struct *napi, struct niu *np,
>  
>  	len += sizeof(*rh);
>  	len = min_t(int, len, sizeof(*rh) + VLAN_ETH_HLEN);
> -	__pskb_pull_tail(skb, len);
> +	if (unlikely(!pskb_may_pull(skb, len))) {
> +		rp->rx_dropped++;

[Severity: Medium]
Could this 64-bit counter update cause a torn read on 32-bit architectures?

Looking at struct rx_ring_info, rx_dropped is declared as a u64. Without
wrapping the increment in u64_stats_update_begin() and 
u64_stats_update_end(), a concurrent reader might fetch half of an old value
and half of a new value during ndo_get_stats64.

> +		kfree_skb(skb);
> +		return num_rcr;
> +	}
>  
>  	rh = (struct rx_pkt_hdr1 *) skb->data;
>  	if (np->dev->features & NETIF_F_RXHASH)

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