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

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

[wei.fang]

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>
---
v2:
Remove max_period and use NETC_TMR_DEFAULT_FIPER directly
---
 drivers/ptp/ptp_netc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
index 59db08e189e6..19ca99d80e95 100644
--- a/drivers/ptp/ptp_netc.c
+++ b/drivers/ptp/ptp_netc.c
@@ -440,7 +440,7 @@ static int net_timer_enable_perout(struct netc_timer *priv,
 	}
 
 	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;
@@ -450,12 +450,12 @@ 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;
 		gclk_period = netc_timer_get_gclk_period(priv);
 		min_period = gclk_period * 4 + integral_period;
-		if (period_ns > max_period || period_ns < min_period) {
-			dev_err(dev, "The period range is %llu ~ %llu\n",
-				min_period, max_period);
+		if (period_ns > NETC_TMR_DEFAULT_FIPER ||
+		    period_ns < min_period) {
+			dev_err(dev, "The period range is %llu ~ %lu\n",
+				min_period, NETC_TMR_DEFAULT_FIPER);
 			err = -EINVAL;
 			goto unlock_spinlock;
 		}
-- 
2.34.1

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

[Abel Vesa]

On 26-08-21 11:24:49, 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>

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>