RE: [PATCH net-next v5 10/14] net: renesas: rswitch: update port HW init

[Michael Dege]

Hello Jakub,

Thank you very much for your comment.

> -----Original Message-----
> From: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Monday, May 25, 2026 10:41 PM
> To: Michael Dege <michael.dege@xxxxxxxxxxx>
> Cc: Jakub Kicinski <kuba@xxxxxxxxxx>; Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>;
> andrew+netdev@xxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; pabeni@xxxxxxxxxx; niklas.soderlund
> <niklas.soderlund@xxxxxxxxxxxx>; paul@xxxxxxxxxxx; richardcochran@xxxxxxxxx; netdev@xxxxxxxxxxxxxxx;
> linux-renesas-soc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next v5 10/14] net: renesas: rswitch: update port HW init
> 
> This is an AI-generated review of your patch. The human sending this email has considered the AI
> review valid, or at least plausible.
> ---
> net: renesas: rswitch: update port HW init
> 
> Certain Ethernet and cpu port settings need to be updated for vlan support.
> 
> > diff --git a/drivers/net/ethernet/renesas/rswitch_main.c
> > b/drivers/net/ethernet/renesas/rswitch_main.c
> > index a445d9a8505d8..13e59d2d08282 100644
> > --- a/drivers/net/ethernet/renesas/rswitch_main.c
> > +++ b/drivers/net/ethernet/renesas/rswitch_main.c
> 
> [ ... ]
> 
> > @@ -895,6 +899,7 @@ static bool rswitch_rx(struct net_device *ndev, int *quota)
> >  	limit = boguscnt;
> >
> >  	desc = &gq->rx_ring[gq->cur];
> > +
> >  	while ((desc->desc.die_dt & DT_MASK) != DT_FEMPTY) {
> >  		dma_rmb();
> >  		skb = rswitch_rx_handle_desc(ndev, gq, desc);
> 
> [Severity: High]
> This is a pre-existing issue, but does this code leak receive buffer fragments allocated via
> netdev_alloc_frag() when packet assembly fails?
> 
> In rswitch_rx_handle_desc(), if build_skb() fails for a start fragment, or if a middle/end fragment
> arrives without a preceding start fragment, the function returns NULL.
> 
> The caller rswitch_rx() then jumps to the out label:
> 
> 	skb = rswitch_rx_handle_desc(ndev, gq, desc);
> 	if (!skb)
> 		goto out;
> 	...
> out:

I added skb_free_frag() here.

Best regards,

Michael
> 	gq->rx_bufs[gq->cur] = NULL;
> 
> This executes gq->rx_bufs[gq->cur] = NULL, discarding the pointer to the memory previously allocated
> by netdev_alloc_frag(). Because the fragment is neither attached to an SKB nor explicitly freed before
> the pointer is zeroed, this appears to cause an unrecoverable memory leak.