linux-kernel - Re: [PATCH wireless v1] wifi: mwifiex: discard erroneous disassoc frames on STA interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aG1lbC4LJDKzMuco@google.com>
Date: Tue, 8 Jul 2025 11:37:32 -0700
From: Brian Norris <briannorris@...omium.org>
To: Vitor Soares <ivitro@...il.com>
Cc: Francesco Dolcini <francesco@...cini.it>,
	Vitor Soares <vitor.soares@...adex.com>,
	linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org,
	David Lin <yu-hao.lin@....com>, stable@...r.kernel.org
Subject: Re: [PATCH wireless v1] wifi: mwifiex: discard erroneous disassoc
 frames on STA interface

Hi Vitor,

On Tue, Jul 01, 2025 at 03:26:43PM +0100, Vitor Soares wrote:
> From: Vitor Soares <vitor.soares@...adex.com>
> 
> 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@...r.kernel.org
> Signed-off-by: Vitor Soares <vitor.soares@...adex.com>
> ---
>  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?

> -			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").

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.

Brian

> +					      priv->curr_bss_params.bss_descriptor.mac_address))
>  				return 0;
>  
>  			if (ieee80211_is_deauth(ieee_hdr->frame_control)) {
> -- 
> 2.34.1
>