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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing PTP clock unregistration for `ccc_gac` (Gen3) devices on the `ravb_open()` error path leads to a Use-After-Free.
- [High] Missing PTP clock driver teardown for `gptp` (Gen2) devices on hardware timeout leads to a Use-After-Free.
- [Medium] Silent loss of PTP clock functionality during TX timeout recovery for `gptp` devices.
--

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

net: ethernet: ravb: Simplify gPTP start and stop

This commit simplifies the gPTP start and stop logic by folding the gptp flag
handling directly into ravb_dmac_init() and ravb_stop_dma(). It aims to
control the clock for gptp devices in tandem with the AVB-DMAC, while leaving
ccc_gac devices tied to device open/close operations. However, this
refactoring appears to introduce regressions in error handling and timeout
recovery paths.

> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index a89f6fe7866fe..2d0022a4f85c3 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);
> +
>  	/* Stop AVB-DMAC process */
>  	return ravb_set_opmode(ndev, CCC_OPC_CONFIG);
>  }

[Severity: High]
Can a hardware timeout cause a use-after-free by skipping the PTP clock
teardown?

Earlier in ravb_stop_dma(), there are multiple ravb_wait() hardware polling
loops. If any of these wait loops time out, the function returns an error
immediately, bypassing this new ravb_ptp_stop() call. During a module unload
or interface close, if ravb_stop_dma() fails, teardown continues but the PTP
clock remains registered. Does this leave the /dev/ptp* device pointing to
freed memory?

[ ... ]
> @@ -1974,9 +1980,6 @@ static int ravb_open(struct net_device *ndev)
>  	return 0;
>  
>  out_ptp_stop:
> -	/* Stop PTP Clock driver */
> -	if (info->gptp || info->ccc_gac)
> -		ravb_ptp_stop(ndev);
>  	ravb_stop_dma(ndev);
>  out_set_reset:
>  	ravb_set_opmode(ndev, CCC_OPC_RESET);

[Severity: High]
Does this error path leak the PTP clock driver for ccc_gac (Gen3) devices?

When ravb_ptp_init() is called successfully but a subsequent step like
ravb_phy_start() fails, the code jumps here to out_ptp_stop. Since
ravb_stop_dma() only stops the clock for gptp devices, the clock remains
registered for ccc_gac devices. If the network device is subsequently freed
by a module unload, could this lead to a use-after-free when userspace
accesses the dangling PTP device?

[ ... ]
> @@ -2056,10 +2055,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]
Does removing this initialization cause a silent loss of PTP clock
functionality during TX timeout recovery?

If ravb_stop_dma() successfully stops the PTP clock but fails at its final
step (ravb_set_opmode()), it returns an error. The error path in
ravb_tx_timeout_work() jumps to the out label, re-enables queues, and
aborts full re-initialization.

Since ravb_ptp_init() is no longer called at the out label, does the PTP
clock remain permanently stopped and unregistered until the interface is
manually brought down and up again?

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