[PATCH v2 0/2] wifi: iwlwifi: Fix GP2 to nanoseconds overflow on 32-bit

[PATCH v2 0/2] wifi: iwlwifi: Fix GP2 to nanoseconds overflow on 32-bit

[Zhan Xusheng]

GP2 timestamps are u32 microsecond values and NSEC_PER_USEC is 1000L, so
on 32-bit five conversions are computed in 32-bit arithmetic and truncated
before being assigned to u64.  A GP2 value above U32_MAX / 1000, i.e.
4.295 s, wraps; GP2 wraps every 2^32 us, so almost its whole range is
affected.

The same files already cast four other conversions, so this is just the
remaining ones:

	gp2_ns = iwl_mvm_ptp_get_adj_time(mvm, (u64)gp2 * NSEC_PER_USEC);

Changes since v1:

 - Added the generated-code evidence below instead of "Found by code
   inspection"
 - Rebased on iwlwifi-next; no code change
 - v1 was sent with the wrong From: address

On i386, iwl_mld_ptp_get_adj_time() changes from a 32-bit product to a
32x32->64 one:

	before: imul   $0x3e8,0x17d8(%eax),%ecx
	after:  mull   0x17d8(%ebx)

0x3e8 is NSEC_PER_USEC.  A standalone _Static_assert() replica confirms the
arithmetic: with gp2 = 4294968 us, the current form gives 704 ns on 32-bit
instead of 4294968000.

mvm/ptp.c:219 already uses a u64 gp2 and is left alone.  The
wrap_counter * IWL_PTP_GP2_WRAP * NSEC_PER_USEC products are fine because
IWL_PTP_GP2_WRAP is 0x100000000ULL.

Built for x86-64 and i386 with IWLMVM=m and IWLMLD=m; checkpatch --strict
is clean on both patches.

v1: https://patch.msgid.link/20260805082515.4136842-1-zhanxusheng@xiaomi.com

Zhan Xusheng (2):
  wifi: iwlwifi: mvm: Fix GP2 to nanoseconds overflow on 32-bit
  wifi: iwlwifi: mld: Fix GP2 to nanoseconds overflow on 32-bit

 drivers/net/wireless/intel/iwlwifi/mld/ptp.c  | 5 +++--
 drivers/net/wireless/intel/iwlwifi/mvm/ptp.c  | 4 ++--
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 4 +++-
 3 files changed, 8 insertions(+), 5 deletions(-)


base-commit: 90e281bd4ea3d19dd6b5af1923551648e477bd26
-- 
2.43.0

[PATCH v2 1/2] wifi: iwlwifi: mvm: Fix GP2 to nanoseconds overflow on 32-bit

[Zhan Xusheng]

scale_update_gp2 and gp2_on_air_rise are u32 GP2 timestamps in
microseconds.  NSEC_PER_USEC is 1000L, so on 32-bit the products are
computed in 32-bit arithmetic and truncated before being widened:

	u64 last_gp2_ns = mvm->ptp_data.scale_update_gp2 * NSEC_PER_USEC;

A GP2 value above (U32_MAX / 1000) us, i.e. 4.295 s, wraps.  GP2 is a
free-running counter that wraps every 2^32 us, so almost its whole range
is affected and the PTP clock jumps.

The neighbouring conversions in the same files already cast:

	gp2_ns = iwl_mvm_ptp_get_adj_time(mvm, (u64)gp2 * NSEC_PER_USEC);

Cast the remaining ones the same way.  The i386 build of
iwl_mvm_ptp_get_adj_time() changes from "imul $0x3e8,...,%ecx" to
"mull", i.e. from a 32-bit product to a 32x32->64 one.

mvm/ptp.c:219 is already u64 and needs no change.

Fixes: a2f49f7d52a9 ("wifi: iwlwifi: mvm: implement PHC clock adjustments")
Fixes: 0e49e940d1bc ("wifi: iwlwifi: mvm: add an option to use ptp clock for rx timestamp")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/ptp.c  | 4 ++--
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
index 49dcb1388007..5f33e2f8fb2a 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
@@ -47,7 +47,7 @@ static void iwl_mvm_ptp_update_new_read(struct iwl_mvm *mvm, u32 gp2)
 u64 iwl_mvm_ptp_get_adj_time(struct iwl_mvm *mvm, u64 base_time_ns)
 {
 	struct ptp_data *data = &mvm->ptp_data;
-	u64 last_gp2_ns = mvm->ptp_data.scale_update_gp2 * NSEC_PER_USEC;
+	u64 last_gp2_ns = (u64)mvm->ptp_data.scale_update_gp2 * NSEC_PER_USEC;
 	u64 res;
 	u64 diff;
 
@@ -259,7 +259,7 @@ static int iwl_mvm_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 	 */
 	gp2 = iwl_mvm_get_systime(mvm);
 	data->scale_update_adj_time_ns =
-		iwl_mvm_ptp_get_adj_time(mvm, gp2 * NSEC_PER_USEC);
+		iwl_mvm_ptp_get_adj_time(mvm, (u64)gp2 * NSEC_PER_USEC);
 	data->scale_update_gp2 = gp2;
 	data->wrap_counter = 0;
 	data->delta = 0;
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index c01c704929ba..e9146c38a81c 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -2021,7 +2021,9 @@ static void iwl_mvm_rx_fill_status(struct iwl_mvm *mvm,
 
 	if (mvm->rx_ts_ptp && mvm->monitor_on) {
 		u64 adj_time =
-			iwl_mvm_ptp_get_adj_time(mvm, phy_data->gp2_on_air_rise * NSEC_PER_USEC);
+			iwl_mvm_ptp_get_adj_time(mvm,
+						 (u64)phy_data->gp2_on_air_rise *
+						 NSEC_PER_USEC);
 
 		rx_status->mactime = div64_u64(adj_time, NSEC_PER_USEC);
 		rx_status->flag |= RX_FLAG_MACTIME_IS_RTAP_TS64;
-- 
2.43.0

Re: [PATCH v2 1/2] wifi: iwlwifi: mvm: Fix GP2 to nanoseconds overflow on 32-bit

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 (1) · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The series fix is incomplete for the mld op-mode.
- [Low] The commit author (From:) is `Zhan Xusheng
  <zhanxusheng1024@gmail.com>`, but the only Signed-off-by is `Zhan…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but in mvm the
  shared PTP state in mvm->ptp_data (last_gp2, wrap_counter,…

--

wifi: iwlwifi: mvm: Fix GP2 to nanoseconds overflow on 32-bit

This patch casts the u32 GP2 microsecond timestamps to u64 before they are
multiplied by NSEC_PER_USEC. The affected functions are
iwl_mvm_ptp_get_adj_time(), iwl_mvm_ptp_adjfine() and
iwl_mvm_rx_fill_status().

On 32-bit kernels these products were computed in 32 bits. They wrapped
for GP2 values above about 4.295 s, which made the PTP clock jump.

> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

[Severity: Low]
This isn't a bug, but the author address (zhanxusheng1024@gmail.com) is
different from the Signed-off-by address (zhanxusheng@xiaomi.com).
checkpatch reports this as a From:/Signed-off-by: email address mismatch.

Could a Signed-off-by be added for the gmail address? Another option is a
From: line with the xiaomi address in the patch body. The follow-up mld
commit in this series has the same mismatch.

> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> index 49dcb1388007b..5f33e2f8fb2a9 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c

[ ... ]

> @@ -259,7 +259,7 @@ static int iwl_mvm_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
>  	 */
>  	gp2 = iwl_mvm_get_systime(mvm);
>  	data->scale_update_adj_time_ns =
> -		iwl_mvm_ptp_get_adj_time(mvm, gp2 * NSEC_PER_USEC);
> +		iwl_mvm_ptp_get_adj_time(mvm, (u64)gp2 * NSEC_PER_USEC);
>  	data->scale_update_gp2 = gp2;
>  	data->wrap_counter = 0;
>  	data->delta = 0;

[Severity: Medium]
This is a pre-existing issue, and this patch did not introduce it. Is
mvm->ptp_data safe from concurrent readers here?

iwl_mvm_ptp_adjfine() updates scale_update_adj_time_ns, scale_update_gp2,
wrap_counter and delta while holding only mvm->mutex.

Some RX paths call iwl_mvm_ptp_get_adj_time() without taking any lock:

  iwl_mvm_rx_fill_status()                (NAPI, rx_ts_ptp && monitor_on)
  iwl_mvm_time_sync_msmt_event()          (RX_HANDLER_SYNC)
  iwl_mvm_time_sync_msmt_confirm_event()  (RX_HANDLER_SYNC)

That function also changes state through iwl_mvm_ptp_update_new_read():

	if (gp2 < mvm->ptp_data.last_gp2) {
		mvm->ptp_data.wrap_counter++;
		...
	}

	mvm->ptp_data.last_gp2 = gp2;

The writers that hold mvm->mutex are iwl_mvm_ptp_adjfine(),
iwl_mvm_ptp_adjtime(), iwl_mvm_ptp_work(), iwl_mvm_ptp_gettime() and
iwl_mvm_phc_get_crosstimestamp(). That mutex does not keep out softirq RX
on other CPUs.

These interleavings look possible:

- RX sees the new scale_update_gp2 before wrap_counter is reset to 0, so
  the timestamp is off by N * 2^32 us.

- Two RSS queues both run the non-atomic wrap_counter++ / last_gp2
  update, so a wrap is counted twice or lost.

- On 32-bit, the u64 fields scale_update_adj_time_ns, scaled_freq and
  delta can tear while adjfine or adjtime writes them.

The mld driver protects the same state with data->lock, using
spin_lock_bh() in mld/ptp.c. Does mvm need similar locking?

> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
> index 7f0b4f5daa214..7a9a63a76f382 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
> @@ -2017,7 +2017,9 @@ static void iwl_mvm_rx_fill_status(struct iwl_mvm *mvm,
>  
>  	if (mvm->rx_ts_ptp && mvm->monitor_on) {
>  		u64 adj_time =
> -			iwl_mvm_ptp_get_adj_time(mvm, phy_data->gp2_on_air_rise * NSEC_PER_USEC);
> +			iwl_mvm_ptp_get_adj_time(mvm,
> +						 (u64)phy_data->gp2_on_air_rise *
> +						 NSEC_PER_USEC);

[Severity: High]
Does the mld op-mode still have this overflow in its RX path?

The follow-up commit "wifi: iwlwifi: mld: Fix GP2 to nanoseconds overflow
on 32-bit" says it fixes the "Same problem as the mvm driver". However, it
only changes the two sites in mld/ptp.c, in iwl_mld_ptp_get_adj_time() and
iwl_mld_ptp_adjfine().

At the end of the series, iwl_mld_rx_fill_status() in mld/rx.c still has
no cast:

drivers/net/wireless/intel/iwlwifi/mld/rx.c:iwl_mld_rx_fill_status() {
    ...
		if (mld->monitor.ptp_time) {
			u64 adj_time =
				iwl_mld_ptp_get_adj_time(mld,
							 phy_data->gp2_on_air_rise *
							 NSEC_PER_USEC);
    ...
}

gp2_on_air_rise is a u32 in struct iwl_mld_rx_phy_data, and NSEC_PER_USEC
is 1000L. On 32-bit the product therefore wraps for any GP2 value above
about 4,294,967 us.

The truncated value is then passed to iwl_mld_ptp_update_new_read() as a
small gp2. It is usually more than IWL_PTP_WRAP_THRESHOLD_USEC below
last_gp2, so it looks like a wraparound:

		mld->ptp_data.wrap_counter++;

This would happen each time the truncated value wraps. It would also
happen each time gettime or crosstimestamp stores a correct, large
last_gp2.

That would make rx_status->mactime wrong. It would also shift later
iwl_mld_ptp_gettime() results by 2^32 us for each spurious wrap.

IWLMLD does not depend on 64BIT, and this path is reachable with the
rx_ts_ptp debugfs knob and a monitor interface. Should the mld follow-up
also cast gp2_on_air_rise to u64 in iwl_mld_rx_fill_status()?

>  
>  		rx_status->mactime = div64_u64(adj_time, NSEC_PER_USEC);
>  		rx_status->flag |= RX_FLAG_MACTIME_IS_RTAP_TS64;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001050103.860584-1-zhanxusheng%40xiaomi.com

[PATCH v2 2/2] wifi: iwlwifi: mld: Fix GP2 to nanoseconds overflow on 32-bit

[Zhan Xusheng]

Same problem as the mvm driver: scale_update_gp2 is a u32 GP2 timestamp
in microseconds and NSEC_PER_USEC is 1000L, so on 32-bit

	u64 scale_time_gp2_ns = mld->ptp_data.scale_update_gp2 * NSEC_PER_USEC;

truncates to 32 bits for any GP2 value above 4.295 s.

iwl_mld_ptp_gettime() and iwl_mld_ptp_get_crosstimestamp() in the same
file already cast to u64 before multiplying.  Do the same here.

Fixes: d1e879ec600f ("wifi: iwlwifi: add iwlmld sub-driver")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 drivers/net/wireless/intel/iwlwifi/mld/ptp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
index 9f7d17dbdee1..643664d35772 100644
--- a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
+++ b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
@@ -49,7 +49,8 @@ static void iwl_mld_ptp_update_new_read(struct iwl_mld *mld, u32 gp2)
 u64 iwl_mld_ptp_get_adj_time(struct iwl_mld *mld, u64 base_time_ns)
 {
 	struct ptp_data *data = &mld->ptp_data;
-	u64 scale_time_gp2_ns = mld->ptp_data.scale_update_gp2 * NSEC_PER_USEC;
+	u64 scale_time_gp2_ns = (u64)mld->ptp_data.scale_update_gp2 *
+				NSEC_PER_USEC;
 	u64 res;
 	u64 diff;
 	s64 scaled_diff;
@@ -149,7 +150,7 @@ static int iwl_mld_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 
 	spin_lock_bh(&data->lock);
 	data->scale_update_adj_time_ns =
-		iwl_mld_ptp_get_adj_time(mld, gp2 * NSEC_PER_USEC);
+		iwl_mld_ptp_get_adj_time(mld, (u64)gp2 * NSEC_PER_USEC);
 	data->scale_update_gp2 = gp2;
 
 	/* scale_update_adj_time_ns now relects the configured delta, the
-- 
2.43.0

Re: [PATCH v2 2/2] wifi: iwlwifi: mld: Fix GP2 to nanoseconds overflow on 32-bit

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete: one u32 GP2 × NSEC_PER_USEC
  multiplication in the mld driver is still unfixed.
- [Low] The commit message is inaccurate and incomplete.

--

wifi: iwlwifi: mld: Fix GP2 to nanoseconds overflow on 32-bit

This patch adds u64 casts to two u32 GP2 * NSEC_PER_USEC multiplications
in the iwlmld PTP code: one in iwl_mld_ptp_get_adj_time() and one in
iwl_mld_ptp_adjfine(). Without the casts, 32-bit kernels compute the
product in 32 bits, and it truncates for GP2 values above about 4.295
seconds.

> iwl_mld_ptp_gettime() and iwl_mld_ptp_get_crosstimestamp() in the same
> file already cast to u64 before multiplying.  Do the same here.

[Severity: Low]
This isn't a bug, but mld/ptp.c has no function called
iwl_mld_ptp_get_crosstimestamp(). Is this meant to be
iwl_mld_phc_get_crosstimestamp()?

The message also describes only the scale_time_gp2_ns line in
iwl_mld_ptp_get_adj_time(). It doesn't mention the second hunk, which
changes the gp2 * NSEC_PER_USEC argument in iwl_mld_ptp_adjfine().

Could the message cover both sites, and the user-visible effect, as the
mvm companion patch does?

> diff --git a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> index 1ac85d6ce1d79..41ba49f525845 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> @@ -59,7 +59,8 @@ static void iwl_mld_ptp_update_new_read(struct iwl_mld *mld, u32 gp2)
>  u64 iwl_mld_ptp_get_adj_time(struct iwl_mld *mld, u64 base_time_ns)
>  {
>  	struct ptp_data *data = &mld->ptp_data;
> -	u64 scale_time_gp2_ns = mld->ptp_data.scale_update_gp2 * NSEC_PER_USEC;
> +	u64 scale_time_gp2_ns = (u64)mld->ptp_data.scale_update_gp2 *
> +				NSEC_PER_USEC;

[Severity: High]
Does one more caller of iwl_mld_ptp_get_adj_time() have the same
truncation? In mld/rx.c, the monitor mode PTP timestamp path in
iwl_mld_rx_fill_status() still does this:

drivers/net/wireless/intel/iwlwifi/mld/rx.c:iwl_mld_rx_fill_status() {
    ...
		if (mld->monitor.ptp_time) {
			u64 adj_time =
				iwl_mld_ptp_get_adj_time(mld,
							 phy_data->gp2_on_air_rise *
							 NSEC_PER_USEC);
    ...
}

gp2_on_air_rise is a u32 in struct iwl_mld_rx_phy_data. On 32-bit, this
multiplication wraps before the result is widened to the u64 argument.

The mvm companion patch, "wifi: iwlwifi: mvm: Fix GP2 to nanoseconds
overflow on 32-bit", casts the matching gp2_on_air_rise site in
iwl_mvm_rx_fill_status().

The truncated value also reaches the shared wraparound tracking, because
iwl_mld_ptp_get_adj_time() does:

	iwl_mld_ptp_update_new_read(mld,
				    div64_u64(base_time_ns, NSEC_PER_USEC));

For example, take gp2_on_air_rise = 10,000,000 us. After the 32-bit wrap
the product is 1,410,065,408 ns, which divides back to 1,410,065 us.

If last_gp2 is around 10,000,000, the gap is larger than
IWL_PTP_WRAP_THRESHOLD_USEC. iwl_mld_ptp_update_new_read() then treats
the frame as a wraparound: it increments wrap_counter and overwrites
last_gp2.

Wouldn't that shift the PHC by 2^32 us for gettime64, adjfine and the
other PTP users, not just the radiotap mactime?

The path can be hit on 32-bit builds with CONFIG_IWLWIFI_DEBUGFS, monitor
mode on, and the rx_ts_ptp debugfs knob set.

Should the rx.c call get the same (u64) cast in this patch?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001050103.860584-1-zhanxusheng%40xiaomi.com