On 12/2/2023 9:56 AM, Jeff Johnson wrote:
> On 6/4/2023 2:11 AM, gregory.greenman@xxxxxxxxx wrote:
>> From: Anjaneyulu <pagadala.yesu.anjaneyulu@xxxxxxxxx>
>>
>> Currently, enum ieee80211_bss_change has more than 32 flags.
>> Change the type of the corresponding variables from u32 to u64.
>>
>> Signed-off-by: Anjaneyulu <pagadala.yesu.anjaneyulu@xxxxxxxxx>
>> Signed-off-by: Gregory Greenman <gregory.greenman@xxxxxxxxx>
>> ---
>> net/mac80211/cfg.c | 79 ++++++++++++++++++--------------------
>> net/mac80211/chan.c | 4 +-
>> net/mac80211/ibss.c | 16 ++++----
>> net/mac80211/ieee80211_i.h | 14 ++++---
>> net/mac80211/iface.c | 4 +-
>> net/mac80211/main.c | 4 +-
>> net/mac80211/mesh.c | 30 ++++++++-------
>> net/mac80211/mesh.h | 19 ++++-----
>> net/mac80211/mesh_plink.c | 37 +++++++++---------
>> net/mac80211/mesh_ps.c | 7 ++--
>> net/mac80211/mlme.c | 12 +++---
>> net/mac80211/ocb.c | 4 +-
>> 12 files changed, 119 insertions(+), 111 deletions(-)
>
> We are finally at the point where we are testing mesh and this patch is
> breaking mesh on 32-bit systems. There seems to be a fundamental issue
> with both the original code and the revised code where bitop operations
> are being used on bitmaps which have not been defined and sized with
> DECLARE_BITMAP.
>
> Note the bitops all use unsigned long * for the bitmap pointer, and
> hence it seems important that all the bitmaps being used with these
> operations use that same underlying type.
>
> The specific code that is causing us issues is in
> ieee80211_mbss_info_change_notify():
> for_each_set_bit(bit, &bits, sizeof(changed) * BITS_PER_BYTE)
> set_bit(bit, ifmsh->mbss_changed);
>
> Here in the current code changed is u64 and ifmsh->mbss_changed is
> unsigned long and hence processing a bit > 32 writes outside
> ifmsh->mbss_changed on a 32-bit system.
>
> At a minimum struct ieee80211_if_mesh::mbss_changed needs to be properly
> sized.
>
> We have validated what I consider to be a quick and dirty change which
> modifies struct ieee80211_if_mesh::mbss_changed from unsigned long to u64.
>
> Do you want that change?
>
> Or do you want to propose an encompassing solution that correctly uses
> unsigned long * and DECLARE_BOTMAP
>
>
> /jeff
>
>
Apologies for reporting an issue that has already been (somewhat) fixed
by 6e48ebffc2db ("wifi: mac80211: fix mesh id corruption on 32 bit
systems"). Issue was found internally in a backported kernel and that
fix was not present.
But note that fix did not use DECLARE_BITMAP which I still think is the
right thing to do everywhere we are using bitops.
/jeff
