Re: [PATCH net-next v5 03/10] net: ethernet: ravb: Simplify gPTP start and stop

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ravb: PTP clock resource leak and Use-After-Free on DMA stop timeout
- [Medium] ravb: PTP timestamping broken after TX timeout recovery
--

commit bd55489575ec22549ad9bb8e730d4b498b5d08a1
Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

net: ethernet: ravb: Simplify gPTP start and stop

This patch reorganizes the initialization and teardown of the gPTP clock
for different device generations. It moves ravb_ptp_init() into
ravb_dmac_init() and ravb_ptp_stop() into ravb_stop_dma() for Gen2 devices.

> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index a89f6fe7866fe..42d208e6b8ecd 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[ ... ]
> @@ -1115,6 +1123,10 @@ static int ravb_stop_dma(struct net_device *ndev)
>  			netdev_err(ndev, "failed to stop AXI BUS\n");
>  	}
>  
> +	/* Stop PTP Clock driver */
> +	if (info->gptp)
> +		ravb_ptp_stop(ndev);
> +

[Severity: High]
If ravb_stop_dma() returns early due to a hardware timeout:

ravb_stop_dma() {
	...
	error = ravb_wait(ndev, TCCR, info->tccr_mask, 0);
	if (error)
		return error;
	...
}

This relocated ravb_ptp_stop() call is skipped. Since ravb_close() ignores
failures from ravb_stop_dma() and proceeds to free the device structures,
will this leave the PTP clock device leaked and tied to soon-to-be-freed
memory, potentially causing a use-after-free when the module is removed?

>  	/* Stop AVB-DMAC process */
>  	return ravb_set_opmode(ndev, CCC_OPC_CONFIG);
>  }
[ ... ]
> @@ -2056,10 +2058,6 @@ static void ravb_tx_timeout_work(struct work_struct *work)
>  	ravb_emac_init(ndev);
>  
>  out:
> -	/* Initialise PTP Clock driver */
> -	if (info->gptp)
> -		ravb_ptp_init(ndev, priv->pdev);
> -
>  	netif_tx_start_all_queues(ndev);
>  
>  out_unlock:

[Severity: Medium]
When ravb_stop_dma() fails late (for example, if ravb_set_opmode() returns
an error at the end of the function), the PTP clock will have already been
stopped by the new code in ravb_stop_dma() above.

If this happens during ravb_tx_timeout_work(), the code jumps to the 'out'
label to safely resume queues, skipping ravb_dmac_init(). Since
ravb_ptp_init() is now folded inside ravb_dmac_init(), doesn't this leave
the PTP clock permanently disabled and break timestamping until the
interface is manually restarted?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921175037.4084310-1-niklas.soderlund+renesas@ragnatech.se?part=3