* [PATCH net] ptp: netc: fix period truncation and potential divide-by-zero in PEROUT
@ 2026-08-18 3:33 wei.fang
2026-08-20 15:56 ` Simon Horman
0 siblings, 1 reply; 5+ messages in thread
From: wei.fang @ 2026-08-18 3:33 UTC (permalink / raw)
To: xiaoning.wang, richardcochran, andrew+netdev, davem, edumazet,
kuba, pabeni, Frank.Li
Cc: wei.fang, imx, netdev, linux-kernel
From: Wei Fang <wei.fang@nxp.com>
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@nxp.com>
---
drivers/ptp/ptp_netc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
index 59db08e189e6..05d606e636b1 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) {
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] ptp: netc: fix period truncation and potential divide-by-zero in PEROUT
2026-08-18 3:33 [PATCH net] ptp: netc: fix period truncation and potential divide-by-zero in PEROUT wei.fang
@ 2026-08-20 15:56 ` Simon Horman
2026-08-21 1:52 ` Wei Fang
0 siblings, 1 reply; 5+ messages in thread
From: Simon Horman @ 2026-08-20 15:56 UTC (permalink / raw)
To: wei.fang
Cc: xiaoning.wang, richardcochran, andrew+netdev, davem, edumazet,
kuba, pabeni, Frank.Li, wei.fang, imx, netdev, linux-kernel
On Tue, Aug 18, 2026 at 11:33:41AM +0800, wei.fang@oss.nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> 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@nxp.com>
If so, then I think NETC_TMR_DEFAULT_FIPER can simply be used in
place of max_period.
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;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH net] ptp: netc: fix period truncation and potential divide-by-zero in PEROUT
2026-08-20 15:56 ` Simon Horman
@ 2026-08-21 1:52 ` Wei Fang
2026-08-21 3:28 ` Abel Vesa
0 siblings, 1 reply; 5+ messages in thread
From: Wei Fang @ 2026-08-21 1:52 UTC (permalink / raw)
To: Simon Horman, Wei Fang (OSS)
Cc: Clark Wang, richardcochran@gmail.com, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, Frank Li, imx@lists.linux.dev,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
NXP Confidential
> On Tue, Aug 18, 2026 at 11:33:41AM +0800, wei.fang@oss.nxp.com wrote:
> > From: Wei Fang <wei.fang@nxp.com>
> >
> > 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@nxp.com>
>
> 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;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] ptp: netc: fix period truncation and potential divide-by-zero in PEROUT
2026-08-21 1:52 ` Wei Fang
@ 2026-08-21 3:28 ` Abel Vesa
2026-08-21 3:32 ` Wei Fang
0 siblings, 1 reply; 5+ messages in thread
From: Abel Vesa @ 2026-08-21 3:28 UTC (permalink / raw)
To: Wei Fang
Cc: Simon Horman, Wei Fang (OSS), Clark Wang,
richardcochran@gmail.com, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, Frank Li, imx@lists.linux.dev,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
On 26-08-21 01:52:37, Wei Fang wrote:
>
> NXP Confidential
Which part is confidential ?
> > On Tue, Aug 18, 2026 at 11:33:41AM +0800, wei.fang@oss.nxp.com wrote:
> > > From: Wei Fang <wei.fang@nxp.com>
> > >
> > > 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@nxp.com>
> >
> > 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. :)
Hopefully, not the lazy part. ;-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH net] ptp: netc: fix period truncation and potential divide-by-zero in PEROUT
2026-08-21 3:28 ` Abel Vesa
@ 2026-08-21 3:32 ` Wei Fang
0 siblings, 0 replies; 5+ messages in thread
From: Wei Fang @ 2026-08-21 3:32 UTC (permalink / raw)
To: Abel Vesa
Cc: Simon Horman, Wei Fang (OSS), Clark Wang,
richardcochran@gmail.com, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, Frank Li, imx@lists.linux.dev,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
>
> On 26-08-21 01:52:37, Wei Fang wrote:
> >
> > NXP Confidential
>
> Which part is confidential ?
Sorry, the company's email policy has been updated, and this label needs
to be manually deleted, but I didn't notice. :(
>
> > > On Tue, Aug 18, 2026 at 11:33:41AM +0800, wei.fang@oss.nxp.com wrote:
> > > > From: Wei Fang <wei.fang@nxp.com>
> > > >
> > > > 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@nxp.com>
> > >
> > > 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. :)
>
> Hopefully, not the lazy part. ;-)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-21 3:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 3:33 [PATCH net] ptp: netc: fix period truncation and potential divide-by-zero in PEROUT wei.fang
2026-08-20 15:56 ` Simon Horman
2026-08-21 1:52 ` Wei Fang
2026-08-21 3:28 ` Abel Vesa
2026-08-21 3:32 ` Wei Fang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox