Re: [PATCH net-next v3 08/10] net: ethernet: ravb: Add callback for gPTP clock index

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling ethtool -T (get_ts_info) when the interface is down causes a kernel panic due to a NULL pointer dereference in ptp_clock_index().
--

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

net: ethernet: ravb: Add callback for gPTP clock index

This patch prepares for adding Gen4 support which has an optional external
gPTP clock. It adds a callback to get the clock index and uses it to
determine if the device should report gPTP support.

> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 7e53eca5327f5..402370e1be634 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1773,8 +1773,13 @@ static int ravb_get_ts_info(struct net_device *ndev,
>  {
>  	struct ravb_private *priv = netdev_priv(ndev);
>  	const struct ravb_hw_info *hw_info = priv->info;
> +	int index = -1;
>  
> -	if (hw_info->ptp) {
> +	if (hw_info->ptp && hw_info->ptp->clock_index)
> +		index = hw_info->ptp->clock_index(ndev);

[Severity: High]
This is a pre-existing issue, but does this safely handle an absent clock
when the interface is down?

If userspace queries timestamping capabilities (e.g., via ethtool -T) while
the interface is down, ravb_get_ts_info() will call the clock_index callback.
However, priv->ptp.clock is only initialized when the interface is brought up
in ravb_ptp_init().

[ ... ]

> @@ -2653,6 +2658,13 @@ static int ravb_gen2_ptp_probe(struct net_device *ndev)
>  	return ravb_compute_gti(ndev, priv->clk);
>  }
>  
> +static int ravb_gen2_ptp_clock_index(struct net_device *ndev)
> +{
> +	struct ravb_private *priv = netdev_priv(ndev);
> +
> +	return ptp_clock_index(priv->ptp.clock);

[Severity: High]
This isn't a bug introduced by this patch, but will this crash if
priv->ptp.clock is NULL?

Because ptp_clock_index() dereferences the passed pointer without validation,
calling it when the interface is down and the clock hasn't been initialized
appears to lead to a NULL pointer dereference.

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