| Message ID | 20241118125640.1110502-1-ih@simonwunderlich.de (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [v2] wifi: mac80211: fix mbss changed flags corruption on 32 bit systems | expand |
On Mon, 18 Nov 2024 at 12:56, Issam Hamdi <ih@simonwunderlich.de> wrote: > --- > Changes in v2: > - Use BITMAP_FROM_U64() to map all the 64 bits. > - Update the commit description. > --- > net/mac80211/mesh.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c > index cb5f16366b9c..e420eb4797a8 100644 > --- a/net/mac80211/mesh.c > +++ b/net/mac80211/mesh.c > @@ -1157,14 +1157,14 @@ void ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata, > u64 changed) > { > struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh; > - unsigned long bits = changed; > + unsigned long bits[] = { BITMAP_FROM_U64(changed) }; Wouldn't it be easier to use this instead: - unsigned long bits = changed; + u64 bits = changed;
On Mon, 2024-11-18 at 13:26 +0000, James Dutton wrote: > > > @@ -1157,14 +1157,14 @@ void ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata, > > u64 changed) > > { > > struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh; > > - unsigned long bits = changed; > > + unsigned long bits[] = { BITMAP_FROM_U64(changed) }; > > Wouldn't it be easier to use this instead: > - unsigned long bits = changed; > + u64 bits = changed; No, that's incorrect for set_bit() etc. at least on 32-bit big-endian systems. Then you can't use for_each_set_bit() etc. johannes
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c index cb5f16366b9c..e420eb4797a8 100644 --- a/net/mac80211/mesh.c +++ b/net/mac80211/mesh.c @@ -1157,14 +1157,14 @@ void ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata, u64 changed) { struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh; - unsigned long bits = changed; + unsigned long bits[] = { BITMAP_FROM_U64(changed) }; u32 bit; if (!bits) return; /* if we race with running work, worst case this work becomes a noop */ - for_each_set_bit(bit, &bits, sizeof(changed) * BITS_PER_BYTE) + for_each_set_bit(bit, bits, sizeof(changed) * BITS_PER_BYTE) set_bit(bit, ifmsh->mbss_changed); set_bit(MESH_WORK_MBSS_CHANGED, &ifmsh->wrkq_flags); wiphy_work_queue(sdata->local->hw.wiphy, &sdata->work);
On 32-bit systems, the size of an unsigned long is 4 bytes, while a u64 is 8 bytes. Therefore, when using or_each_set_bit(bit, &bits, sizeof(changed) * BITS_PER_BYTE), the code is incorrectly searching for a bit in a 32-bit variable that is expected to be 64 bits in size, leading to incorrect bit finding. Solution: Ensure that the size of the bits variable is correctly adjusted for each architecture by use BITMAP_FROM_U64(). Call Trace: ? show_regs+0x54/0x58 ? __warn+0x6b/0xd4 ? ieee80211_link_info_change_notify+0xcc/0xd4 [mac80211] ? report_bug+0x113/0x150 ? exc_overflow+0x30/0x30 ? handle_bug+0x27/0x44 ? exc_invalid_op+0x18/0x50 ? handle_exception+0xf6/0xf6 ? exc_overflow+0x30/0x30 ? ieee80211_link_info_change_notify+0xcc/0xd4 [mac80211] ? exc_overflow+0x30/0x30 ? ieee80211_link_info_change_notify+0xcc/0xd4 [mac80211] ? ieee80211_mesh_work+0xff/0x260 [mac80211] ? cfg80211_wiphy_work+0x72/0x98 [cfg80211] ? process_one_work+0xf1/0x1fc ? worker_thread+0x2c0/0x3b4 ? kthread+0xc7/0xf0 ? mod_delayed_work_on+0x4c/0x4c ? kthread_complete_and_exit+0x14/0x14 ? ret_from_fork+0x24/0x38 ? kthread_complete_and_exit+0x14/0x14 ? ret_from_fork_asm+0xf/0x14 ? entry_INT80_32+0xf0/0xf0 Reported-by: Kretschmer Mathias <mathias.kretschmer@fit.fraunhofer.de> Signed-off-by: Issam Hamdi <ih@simonwunderlich.de> --- Changes in v2: - Use BITMAP_FROM_U64() to map all the 64 bits. - Update the commit description. --- net/mac80211/mesh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) base-commit: dfc14664794a4706e0c2186a0c082386e6b14c4d