On Tue, 2025-07-08 at 11:37 -0700, Brian Norris wrote:
> Hi Vitor,
>
> On Tue, Jul 01, 2025 at 03:26:43PM +0100, Vitor Soares wrote:
> > From: Vitor Soares <vitor.soares@xxxxxxxxxxx>
> >
> > When operating in concurrent STA/AP mode with host MLME enabled,
> > the firmware incorrectly sends disassociation frames to the STA
> > interface when clients disconnect from the AP interface.
> > This causes kernel warnings as the STA interface processes
> > disconnect events that don't apply to it:
> >
> > [ 1303.240540] WARNING: CPU: 0 PID: 513 at net/wireless/mlme.c:141
> > cfg80211_process_disassoc+0x78/0xec [cfg80211]
> > [ 1303.250861] Modules linked in: 8021q garp stp mrp llc rfcomm bnep
> > btnxpuart nls_iso8859_1 nls_cp437 onboard_us
> > [ 1303.327651] CPU: 0 UID: 0 PID: 513 Comm: kworker/u9:2 Not tainted 6.16.0-
> > rc1+ #3 PREEMPT
> > [ 1303.335937] Hardware name: Toradex Verdin AM62 WB on Verdin Development
> > Board (DT)
> > [ 1303.343588] Workqueue: MWIFIEX_RX_WORK_QUEUE mwifiex_rx_work_queue
> > [mwifiex]
> > [ 1303.350856] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=-
> > -)
> > [ 1303.357904] pc : cfg80211_process_disassoc+0x78/0xec [cfg80211]
> > [ 1303.364065] lr : cfg80211_process_disassoc+0x70/0xec [cfg80211]
> > [ 1303.370221] sp : ffff800083053be0
> > [ 1303.373590] x29: ffff800083053be0 x28: 0000000000000000 x27:
> > 0000000000000000
> > [ 1303.380855] x26: 0000000000000000 x25: 00000000ffffffff x24:
> > ffff000002c5b8ae
> > [ 1303.388120] x23: ffff000002c5b884 x22: 0000000000000001 x21:
> > 0000000000000008
> > [ 1303.395382] x20: ffff000002c5b8ae x19: ffff0000064dd408 x18:
> > 0000000000000006
> > [ 1303.402646] x17: 3a36333a61623a30 x16: 32206d6f72662063 x15:
> > ffff800080bfe048
> > [ 1303.409910] x14: ffff000003625300 x13: 0000000000000001 x12:
> > 0000000000000000
> > [ 1303.417173] x11: 0000000000000002 x10: ffff000003958600 x9 :
> > ffff000003625300
> > [ 1303.424434] x8 : ffff00003fd9ef40 x7 : ffff0000039fc280 x6 :
> > 0000000000000002
> > [ 1303.431695] x5 : ffff0000038976d4 x4 : 0000000000000000 x3 :
> > 0000000000003186
> > [ 1303.438956] x2 : 000000004836ba20 x1 : 0000000000006986 x0 :
> > 00000000d00479de
> > [ 1303.446221] Call trace:
> > [ 1303.448722] cfg80211_process_disassoc+0x78/0xec [cfg80211] (P)
> > [ 1303.454894] cfg80211_rx_mlme_mgmt+0x64/0xf8 [cfg80211]
> > [ 1303.460362] mwifiex_process_mgmt_packet+0x1ec/0x460 [mwifiex]
> > [ 1303.466380] mwifiex_process_sta_rx_packet+0x1bc/0x2a0 [mwifiex]
> > [ 1303.472573] mwifiex_handle_rx_packet+0xb4/0x13c [mwifiex]
> > [ 1303.478243] mwifiex_rx_work_queue+0x158/0x198 [mwifiex]
> > [ 1303.483734] process_one_work+0x14c/0x28c
> > [ 1303.487845] worker_thread+0x2cc/0x3d4
> > [ 1303.491680] kthread+0x12c/0x208
> > [ 1303.495014] ret_from_fork+0x10/0x20
> >
> > Add validation in the STA receive path to verify that disassoc/deauth
> > frames originate from the connected AP. Frames that fail this check
> > are discarded early, preventing them from reaching the MLME layer and
> > triggering WARN_ON().
> >
> > This filtering logic is similar with that used in the
> > ieee80211_rx_mgmt_disassoc() function in mac80211, which drops
> > disassoc frames that don't match the current BSSID
> > (!ether_addr_equal(mgmt->bssid, sdata->vif.cfg.ap_addr)), ensuring
> > only relevant frames are processed.
> >
> > Tested on:
> > - 8997 with FW 16.68.1.p197
> >
> > Fixes: 36995892c271 ("wifi: mwifiex: add host mlme for client mode")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Vitor Soares <vitor.soares@xxxxxxxxxxx>
> > ---
> > drivers/net/wireless/marvell/mwifiex/util.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/util.c
> > b/drivers/net/wireless/marvell/mwifiex/util.c
> > index 4c5b1de0e936..6882e90e90b2 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/util.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/util.c
> > @@ -459,7 +459,9 @@ mwifiex_process_mgmt_packet(struct mwifiex_private
> > *priv,
> > "auth: receive authentication from
> > %pM\n",
> > ieee_hdr->addr3);
> > } else {
>
> How about the other leg of this 'else' (ieee80211_is_auth())? Is it
> possible for these spurious frames to accidentally look like our AUTH
> frames?
I haven't seen spurious frames matching the AUTH path so far. Also, if there's
no pending authentication on the STA, those frames should be already ignored.
>
> > - if (!priv->wdev.connected)
> > + if (!priv->wdev.connected ||
> > + !ether_addr_equal(ieee_hdr->addr3,
>
> "addr3" doesn't make it totally obvious to me what this actually means,
> nor that it's correct. Would ieee80211_get_SA(ieee_hdr) be equivalent?
> That seems a bit more descriptive. Or else maybe a short comment (e.g.,
> "ignore spurious management frames from other BSSIDs").
In this case, ieee80211_get_SA() wouldn't be equivalent and as far as I know,
there isn't a helper like ieee80211_get_SA() available.
This is an IBSS/Ad-hoc frame (To DS = 0, From DS = 0), so addr3 contains the
BSSID, which is what we need to match against (same as
ieee80211_rx_mgmt_disassoc()).
Since the patch was already merged, if you think a clarifying comment would
still be valuable, I can send a follow-up patch to add it.
>
> It seems like it's correct, because this block already filters for
> specific frame types (auth, deauth, disassoc), but in isolation, it's
> not the most readable.
>
Yes, I agree with you.
thanks,
Vitor
> Brian
>
> > + priv-
> > >curr_bss_params.bss_descriptor.mac_address))
> > return 0;
> >
> > if (ieee80211_is_deauth(ieee_hdr->frame_control)) {
> > --
> > 2.34.1
> >
