Re: [PATCH net] ptp: netc: fix period truncation and potential divide-by-zero in PEROUT

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Division by zero in `roundup_u64()` when `integral_period` evaluates to 0 due to clock frequency adjustments.
- [Medium] Missing validation of `clk_get_rate()` allows a division by zero if the reference clock rate is 0.
--

--- Patch [1]: [PATCH net] ptp: netc: fix period truncation and potential divide-by-zero in PEROUT ---
commit 29d635c27b8bca8b216f11e24d3881ee5aec72c6
Author: Wei Fang <wei.fang@nxp.com>

ptp: netc: fix period truncation and potential divide-by-zero in PEROUT

This commit caps max_period at NETC_TMR_DEFAULT_FIPER to prevent a period
truncation bug in net_timer_enable_perout() that causes silent truncation
and potential divide-by-zero issues in PEROUT.

> diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
> index 59db08e189e6e..05d606e636b1a 100644
> --- a/drivers/ptp/ptp_netc.c
> +++ b/drivers/ptp/ptp_netc.c
> @@ -450,7 +450,7 @@ static int net_timer_enable_perout(struct netc_timer *priv,
>  		period_ns = timespec64_to_ns(&period);
>  
>  		integral_period = netc_timer_get_integral_period(priv);
> -		max_period = (u64)NETC_TMR_DEFAULT_FIPER + integral_period;
> +		max_period = NETC_TMR_DEFAULT_FIPER;
>  		gclk_period = netc_timer_get_gclk_period(priv);
>  		min_period = gclk_period * 4 + integral_period;
>  		if (period_ns > max_period || period_ns < min_period) {

[Severity: High]
This isn't a bug introduced by this patch, but can a zero integral_period
trigger a divide-by-zero panic in roundup_u64()?

Looking at netc_timer_set_pps_alarm() and netc_timer_set_perout_alarm():

drivers/ptp/ptp_netc.c:netc_timer_set_pps_alarm() {
	...
	alarm = netc_timer_cur_time_read(priv) +  NSEC_PER_MSEC;
	alarm = roundup_u64(alarm, NSEC_PER_SEC);
	alarm = roundup_u64(alarm, integral_period);
	...
}

drivers/ptp/ptp_netc.c:netc_timer_set_perout_alarm() {
	...
	alarm = roundup_u64(stime - period, integral_period);
	netc_timer_alarm_write(priv, alarm, pp->alarm_id);
}

If the hardware clock frequency is fast enough (e.g., 600 MHz, giving a base
period of 1.666 ns), a large negative adjustment (e.g., -40% or -400,000,000
ppb) during a PTP adjfine ioctl will reduce the new_period below 1.0 ns.

When this happens, upper_32_bits(new_period) evaluates to 0, setting the
integral_period to 0.

netc_timer_adjust_period() then disables and re-enables the FIPER hardware,
invoking netc_timer_enable_fiper(), which sequentially calls
netc_timer_set_pps_alarm() and netc_timer_set_perout_alarm().

Since roundup_u64() performs a division via DIV_U64_ROUND_UP, passing 0
causes an immediate kernel panic. This path is reachable via the standard
PTP adjfine ioctl by userspace processes with CAP_SYS_TIME.

[Severity: Medium]
This is a pre-existing issue, but does this code validate the return value
from clk_get_rate()?

drivers/ptp/ptp_netc.c:netc_timer_get_reference_clk_source() {
	...
		if (clk) {
			priv->clk_freq = clk_get_rate(clk);
			priv->clk_select = i ? NETC_TMR_EXT_OSC :
					       NETC_TMR_CCM_TIMER1;
			break;
		}
	}

	/* The period is a 64-bit number, the high 32-bit is the integer
	 * part of the period, the low 32-bit is the fractional part of
	 * the period. In order to get the desired 32-bit fixed-point
	 * format, multiply the numerator of the fraction by 2^32.
	 */
	priv->period = div_u64((u64)NSEC_PER_SEC << 32, priv->clk_freq);
	...
}

If the clock is misconfigured or specified as a dummy clock in the device tree,
clk_get_rate() returns 0.

Passing 0 as the divisor to div_u64() will trigger an unhandled divide-by-zero
exception during the driver's probe sequence.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818033342.3154990-1-wei.fang@oss.nxp.com?part=1