RE: [PATCH net] ptp: netc: fix period truncation and potential divide-by-zero in PEROUT
From: Wei Fang
Date: Thu Aug 20 2026 - 21:52:59 EST
NXP Confidential
> On Tue, Aug 18, 2026 at 11:33:41AM +0800, wei.fang@xxxxxxxxxxx wrote:
> > From: Wei Fang <wei.fang@xxxxxxx>
> >
> > The max_period bound in net_timer_enable_perout() was computed as:
> >
> > max_period = (u64)NETC_TMR_DEFAULT_FIPER + integral_period;
> >
> > which exceeds U32_MAX when integral_period > 0 (e.g. 0x100000002 for
> > the default 333333333 Hz clock). A period_ns that passes this check but
> > exceeds U32_MAX is then silently truncated when stored into the u32
> > struct netc_pp::period field.
> >
> > A truncated value of zero can reach netc_timer_set_perout_alarm(), where
> > the local u32 period variable would also be 0, causing a divide-by-zero
> > in roundup_u64(delta, period) whenever the stime < min_time branch is
> > taken (which always happens for a start time of {0, 0}).
> >
> > Additionally, netc_timer_enable_periodic_pulse() and
> > netc_timer_enable_fiper() both compute:
> >
> > fiper = pp->period - integral_period;
> >
> > A zero pp->period results in an unsigned wraparound to 0xFFFFFFFD,
> > mis-programming the FIPER hardware register.
> >
> > Fix all three issues by capping max_period at NETC_TMR_DEFAULT_FIPER
> > (0xFFFFFFFF). This ensures that any period_ns passing the range check
> > fits in a u32 without truncation, so the stored value is always valid
> > and non-zero. The accepted range is reduced by integral_period ns
> > (typically only a few nanoseconds), which is negligible in practice.
> >
> > Fixes: 671e266835b8 ("ptp: netc: add periodic pulse output support")
> > Signed-off-by: Wei Fang <wei.fang@xxxxxxx>
>
> If so, then I think NETC_TMR_DEFAULT_FIPER can simply be used in
> place of max_period.
Yes, I also considered this modification, but ultimately chose this lazy
approach. Since you brought it up, I will improve it in the next version.
Thanks. :)
>
> Something like this (completely untested!):
>
> diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
> index 28889b7b3e15..66d1f0532585 100644
> --- a/drivers/ptp/ptp_netc.c
> +++ b/drivers/ptp/ptp_netc.c
> @@ -434,22 +434,22 @@
> }
>
> if (on) {
> - u64 period_ns, gclk_period, max_period, min_period;
> + u64 period_ns, gclk_period, min_period;
> struct timespec64 period, stime;
> u32 integral_period;
> int alarm_id;
>
> period.tv_sec = rq->perout.period.sec;
> period.tv_nsec = rq->perout.period.nsec;
> period_ns = timespec64_to_ns(&period);
>
> integral_period = netc_timer_get_integral_period(priv);
> - 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) {
> + if (period_ns > NETC_TMR_DEFAULT_FIPER ||
> + period_ns < min_period) {
> dev_err(dev, "The period range is %llu ~ %lu\n",
> - min_period, max_period);
> + min_period, NETC_TMR_DEFAULT_FIPER);
> err = -EINVAL;
> goto unlock_spinlock;
> }