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 <[email protected]> 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-9512b0fb9...@toxicpanda.com?part=1
