2023-09-13 05:04:09

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH wireless] wifi: mac80211: fix mesh id corruption on 32 bit systems

Since the changed field size was increased to u64, mesh_bss_info_changed
pulls invalid bits from the first 3 bytes of the mesh id, clears them, and
passes them on to ieee80211_link_info_change_notify, because
ifmsh->mbss_changed was not updated to match its size.
Fix this by turning into ifmsh->mbss_changed into an unsigned long array with
64 bit size.

Fixes: 15ddba5f4311 ("wifi: mac80211: consistently use u64 for BSS changes")
Reported-by: Thomas Hühn <[email protected]>
Signed-off-by: Felix Fietkau <[email protected]>
---
net/mac80211/ieee80211_i.h | 2 +-
net/mac80211/mesh.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index b8465d205076..3c5dbf95685d 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -682,7 +682,7 @@ struct ieee80211_if_mesh {
struct timer_list mesh_path_root_timer;

unsigned long wrkq_flags;
- unsigned long mbss_changed;
+ unsigned long mbss_changed[64 / BITS_PER_LONG];

bool userspace_handles_dfs;

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 0d0fbae51b61..092a1dc7314d 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1175,7 +1175,7 @@ void ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata,

/* if we race with running work, worst case this work becomes a noop */
for_each_set_bit(bit, &bits, sizeof(changed) * BITS_PER_BYTE)
- set_bit(bit, &ifmsh->mbss_changed);
+ set_bit(bit, ifmsh->mbss_changed);
set_bit(MESH_WORK_MBSS_CHANGED, &ifmsh->wrkq_flags);
wiphy_work_queue(sdata->local->hw.wiphy, &sdata->work);
}
@@ -1257,7 +1257,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)

/* clear any mesh work (for next join) we may have accrued */
ifmsh->wrkq_flags = 0;
- ifmsh->mbss_changed = 0;
+ memset(ifmsh->mbss_changed, 0, sizeof(ifmsh->mbss_changed));

local->fif_other_bss--;
atomic_dec(&local->iff_allmultis);
@@ -1722,9 +1722,9 @@ static void mesh_bss_info_changed(struct ieee80211_sub_if_data *sdata)
u32 bit;
u64 changed = 0;

- for_each_set_bit(bit, &ifmsh->mbss_changed,
+ for_each_set_bit(bit, ifmsh->mbss_changed,
sizeof(changed) * BITS_PER_BYTE) {
- clear_bit(bit, &ifmsh->mbss_changed);
+ clear_bit(bit, ifmsh->mbss_changed);
changed |= BIT(bit);
}

--
2.41.0


2023-09-15 02:03:51

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH wireless] wifi: mac80211: fix mesh id corruption on 32 bit systems



> -----Original Message-----
> From: Felix Fietkau <[email protected]>
> Sent: Wednesday, September 13, 2023 1:02 PM
> To: [email protected]
> Cc: [email protected]; [email protected]; Thomas Hühn
> <[email protected]>
> Subject: [PATCH wireless] wifi: mac80211: fix mesh id corruption on 32 bit systems
>
> Since the changed field size was increased to u64, mesh_bss_info_changed
> pulls invalid bits from the first 3 bytes of the mesh id, clears them, and
> passes them on to ieee80211_link_info_change_notify, because
> ifmsh->mbss_changed was not updated to match its size.
> Fix this by turning into ifmsh->mbss_changed into an unsigned long array with
> 64 bit size.
>
> Fixes: 15ddba5f4311 ("wifi: mac80211: consistently use u64 for BSS changes")
> Reported-by: Thomas Hühn <[email protected]>
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
> net/mac80211/ieee80211_i.h | 2 +-
> net/mac80211/mesh.c | 8 ++++----
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index b8465d205076..3c5dbf95685d 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -682,7 +682,7 @@ struct ieee80211_if_mesh {
> struct timer_list mesh_path_root_timer;
>
> unsigned long wrkq_flags;
> - unsigned long mbss_changed;
> + unsigned long mbss_changed[64 / BITS_PER_LONG];

mbss_changed is a bitmap, so

DECLARE_BITMAP(mbss_changed, 64);

>
> bool userspace_handles_dfs;
>
> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
> index 0d0fbae51b61..092a1dc7314d 100644
> --- a/net/mac80211/mesh.c
> +++ b/net/mac80211/mesh.c
> @@ -1175,7 +1175,7 @@ void ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata,
>
> /* if we race with running work, worst case this work becomes a noop */
> for_each_set_bit(bit, &bits, sizeof(changed) * BITS_PER_BYTE)
> - set_bit(bit, &ifmsh->mbss_changed);
> + set_bit(bit, ifmsh->mbss_changed);
> set_bit(MESH_WORK_MBSS_CHANGED, &ifmsh->wrkq_flags);
> wiphy_work_queue(sdata->local->hw.wiphy, &sdata->work);
> }
> @@ -1257,7 +1257,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
>
> /* clear any mesh work (for next join) we may have accrued */
> ifmsh->wrkq_flags = 0;
> - ifmsh->mbss_changed = 0;
> + memset(ifmsh->mbss_changed, 0, sizeof(ifmsh->mbss_changed));

bitmap_zero(ifmsh->mbss_changed, 64);

bitmap_zero() can choose just '=0' or memset(0) according to BITS_PER_LONG.

>
> local->fif_other_bss--;
> atomic_dec(&local->iff_allmultis);
> @@ -1722,9 +1722,9 @@ static void mesh_bss_info_changed(struct ieee80211_sub_if_data *sdata)
> u32 bit;
> u64 changed = 0;
>
> - for_each_set_bit(bit, &ifmsh->mbss_changed,
> + for_each_set_bit(bit, ifmsh->mbss_changed,
> sizeof(changed) * BITS_PER_BYTE) {
> - clear_bit(bit, &ifmsh->mbss_changed);
> + clear_bit(bit, ifmsh->mbss_changed);
> changed |= BIT(bit);
> }
>
> --
> 2.41.0