Re: [PATCH] net: ti: icssg-prueth: fix missing data copy and wrong recycle in ZC RX dispatch

[David CARLIER <devnexen@gmail.com>]

Hi,

On Wed, 25 Mar 2026 at 09:14, Meghana Malladi <m-malladi@ti.com> wrote:
>
> Hi David,
>
> On 3/25/26 02:44, David Carlier wrote:
> > emac_dispatch_skb_zc() allocates a new skb via napi_alloc_skb() but
> > never copies the packet data from the XDP buffer into it. The skb is
> > passed up the stack containing uninitialized heap memory instead of
> > the actual received packet, leaking kernel heap contents to userspace.
> >
> > Add the missing memcpy from xdp->data into the skb data area.
> >
> > Additionally, remove the skb_mark_for_recycle() call since the skb is
> > backed by the NAPI page frag allocator, not page_pool. Marking a
> > non-page_pool skb for recycle causes the free path to return pages to
> > a page_pool that does not own them, corrupting page_pool state.
> >
> > The non-ZC path (emac_rx_packet) does not have these issues because it
> > uses napi_build_skb() to wrap the existing page_pool page directly,
> > requiring no copy, and correctly marks for recycle since the page comes
> > from page_pool_dev_alloc_pages().
> >
>
> Thanks for the patch. I wonder how were you able to catch this bug?

By code review, comparing emac_dispatch_skb_zc() with the non-ZC path
emac_rx_packet() and noticing the ZC path allocates a separate skb but
never copies data into it.

>
> > Fixes: 7a64bb388df3 ("net: ti: icssg-prueth: Add AF_XDP zero copy for RX")
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
> >   drivers/net/ethernet/ti/icssg/icssg_common.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> > index fd4e7622f123..9638a03bebb4 100644
> > --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> > +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> > @@ -902,6 +902,7 @@ static void emac_dispatch_skb_zc(struct prueth_emac *emac, struct xdp_buff *xdp,
> >
> >       skb_reserve(skb, headroom);
> >       skb_put(skb, pkt_len);
> > +     memcpy(skb->data, xdp->data, pkt_len);
>
> Why not use skb_copy_to_linear_data() ?

Ah, that is a good suggestion, did not know it.

>
> >       skb->dev = ndev;
> >
> >       /* RX HW timestamp */
> > @@ -912,7 +913,6 @@ static void emac_dispatch_skb_zc(struct prueth_emac *emac, struct xdp_buff *xdp,
> >               skb->offload_fwd_mark = emac->offload_fwd_mark;
> >       skb->protocol = eth_type_trans(skb, ndev);
> >
> > -     skb_mark_for_recycle(skb);
> >       napi_gro_receive(&emac->napi_rx, skb);
> >       ndev->stats.rx_bytes += pkt_len;
> >       ndev->stats.rx_packets++;
>