Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH wireless-next 0/3] wifi: nxpwifi: three fixes mwifiex already made
@ 2026-08-13  8:23 Linmao Li
  2026-08-13  8:23 ` [PATCH wireless-next 1/3] wifi: nxpwifi: wait for the wakeup timer before the adapter is freed Linmao Li
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Linmao Li @ 2026-08-13  8:23 UTC (permalink / raw)
  To: Jeff Chen
  Cc: Francesco Dolcini, Johannes Berg, linux-wireless, linux-kernel,
	Linmao Li

nxpwifi was copied from mwifiex before it was submitted, and some of
the bugs mwifiex has fixed since then are still here.  Each of these
three is the nxpwifi side of a fix mwifiex already carries; the upstream
commit is named in each patch.

Patch 1: a running wakeup timer callback keeps using the adapter after
it is freed (mwifiex: ae5e95d41574).

Patch 2: the A-MSDU buffer is leaked when the RA list disappears in the
middle of aggregation (mwifiex: 990a73dec3fd).

Patch 3: survey data is reported out of memory that was never
initialised (mwifiex: 0e20450829ca).

The three are independent and touch different files, so they can be
taken in any order.

Found by inspection; there is no IW61x hardware here to test on.

Linmao Li (3):
  wifi: nxpwifi: wait for the wakeup timer before the adapter is freed
  wifi: nxpwifi: free the aggregation buffer when the RA list disappears
  wifi: nxpwifi: zero the channel statistics array

 drivers/net/wireless/nxp/nxpwifi/11n_aggr.c | 1 +
 drivers/net/wireless/nxp/nxpwifi/cfg80211.c | 5 +++--
 drivers/net/wireless/nxp/nxpwifi/init.c     | 2 +-
 drivers/net/wireless/nxp/nxpwifi/main.c     | 4 ++--
 4 files changed, 7 insertions(+), 5 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH wireless-next 1/3] wifi: nxpwifi: wait for the wakeup timer before the adapter is freed
  2026-08-13  8:23 [PATCH wireless-next 0/3] wifi: nxpwifi: three fixes mwifiex already made Linmao Li
@ 2026-08-13  8:23 ` Linmao Li
  2026-09-01 10:13   ` Jeff Chen
  2026-08-13  8:23 ` [PATCH wireless-next 2/3] wifi: nxpwifi: free the aggregation buffer when the RA list disappears Linmao Li
  2026-08-13  8:23 ` [PATCH wireless-next 3/3] wifi: nxpwifi: zero the channel statistics array Linmao Li
  2 siblings, 1 reply; 7+ messages in thread
From: Linmao Li @ 2026-08-13  8:23 UTC (permalink / raw)
  To: Jeff Chen
  Cc: Francesco Dolcini, Johannes Berg, linux-wireless, linux-kernel,
	Linmao Li

nxpwifi_adapter_cleanup() stops adapter->wakeup_timer with
timer_delete(), which does not wait for a running callback.  The
callback goes on using the adapter: wakeup_timer_fn() sets
adapter->hw_status, walks the command queues through
nxpwifi_cancel_all_pending_cmd() and calls adapter->if_ops.card_reset().
Both paths that reach nxpwifi_adapter_cleanup() free the adapter right
afterwards, through nxpwifi_free_adapter() in nxpwifi_remove_card() and
in the nxpwifi_add_card() error unwind.

Use timer_delete_sync() so the callback has finished before the adapter
is released.

mwifiex fixed the same issue in commit ae5e95d41574 ("wifi: mwifiex: fix
use-after-free in mwifiex_adapter_cleanup()").

Fixes: 73b01e57ed3e ("wifi: nxp: add nxpwifi driver for IW61x")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/net/wireless/nxp/nxpwifi/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/nxp/nxpwifi/init.c b/drivers/net/wireless/nxp/nxpwifi/init.c
index b128fc9fe31a2..69b27fce5ce65 100644
--- a/drivers/net/wireless/nxp/nxpwifi/init.c
+++ b/drivers/net/wireless/nxp/nxpwifi/init.c
@@ -328,7 +328,7 @@ static void nxpwifi_invalidate_lists(struct nxpwifi_adapter *adapter)
 static void
 nxpwifi_adapter_cleanup(struct nxpwifi_adapter *adapter)
 {
-	timer_delete(&adapter->wakeup_timer);
+	timer_delete_sync(&adapter->wakeup_timer);
 	nxpwifi_cancel_all_pending_cmd(adapter);
 	wake_up_interruptible(&adapter->cmd_wait_q.wait);
 	wake_up_interruptible(&adapter->hs_activate_wait_q);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH wireless-next 2/3] wifi: nxpwifi: free the aggregation buffer when the RA list disappears
  2026-08-13  8:23 [PATCH wireless-next 0/3] wifi: nxpwifi: three fixes mwifiex already made Linmao Li
  2026-08-13  8:23 ` [PATCH wireless-next 1/3] wifi: nxpwifi: wait for the wakeup timer before the adapter is freed Linmao Li
@ 2026-08-13  8:23 ` Linmao Li
  2026-09-08 10:21   ` Jeff Chen
  2026-08-13  8:23 ` [PATCH wireless-next 3/3] wifi: nxpwifi: zero the channel statistics array Linmao Li
  2 siblings, 1 reply; 7+ messages in thread
From: Linmao Li @ 2026-08-13  8:23 UTC (permalink / raw)
  To: Jeff Chen
  Cc: Francesco Dolcini, Johannes Berg, linux-wireless, linux-kernel,
	Linmao Li

nxpwifi_11n_aggregate_pkt() drops ra_list_spinlock while it copies each
subframe, so it rechecks the RA list after taking the lock again.  The
check inside the aggregation loop returns without releasing the
skb_aggr it has been filling, leaking one tx_buf_size buffer along with
the subframes already aggregated into it.

Release skb_aggr there, the way the same check on the -EBUSY path
already does.

mwifiex fixed the same issue in commit 990a73dec3fd ("wifi: mwifiex: Fix
memory leak in mwifiex_11n_aggregate_pkt()").

Fixes: 73b01e57ed3e ("wifi: nxp: add nxpwifi driver for IW61x")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/net/wireless/nxp/nxpwifi/11n_aggr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/nxp/nxpwifi/11n_aggr.c b/drivers/net/wireless/nxp/nxpwifi/11n_aggr.c
index be7080f2a6ce7..54933c42c960d 100644
--- a/drivers/net/wireless/nxp/nxpwifi/11n_aggr.c
+++ b/drivers/net/wireless/nxp/nxpwifi/11n_aggr.c
@@ -168,6 +168,7 @@ nxpwifi_11n_aggregate_pkt(struct nxpwifi_private *priv,
 
 		if (!nxpwifi_is_ralist_valid(priv, pra_list, ptrindex)) {
 			spin_unlock_bh(&priv->wmm.ra_list_spinlock);
+			nxpwifi_write_data_complete(adapter, skb_aggr, 1, -1);
 			return -ENOENT;
 		}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH wireless-next 3/3] wifi: nxpwifi: zero the channel statistics array
  2026-08-13  8:23 [PATCH wireless-next 0/3] wifi: nxpwifi: three fixes mwifiex already made Linmao Li
  2026-08-13  8:23 ` [PATCH wireless-next 1/3] wifi: nxpwifi: wait for the wakeup timer before the adapter is freed Linmao Li
  2026-08-13  8:23 ` [PATCH wireless-next 2/3] wifi: nxpwifi: free the aggregation buffer when the RA list disappears Linmao Li
@ 2026-08-13  8:23 ` Linmao Li
  2026-09-08 10:31   ` Jeff Chen
  2 siblings, 1 reply; 7+ messages in thread
From: Linmao Li @ 2026-08-13  8:23 UTC (permalink / raw)
  To: Jeff Chen
  Cc: Francesco Dolcini, Johannes Berg, linux-wireless, linux-kernel,
	Linmao Li

adapter->chan_stats comes from vmalloc(), which does not clear the
memory, and nxpwifi_cfg80211_dump_survey() reads it as soon as user
space asks for survey data.  For the entries no scan has filled in, a
non-zero cca_scan_dur passes the validity check and stale bytes are
reported to user space as noise, time and time_busy.

Use kcalloc() instead.  The array holds two entries per supported
channel, small enough not to need vmalloc(), and the two callers change
to kfree() accordingly.

mwifiex fixed the same issue in commit 0e20450829ca ("wifi: mwifiex:
Initialize the chan_stats array to zero").

Fixes: 73b01e57ed3e ("wifi: nxp: add nxpwifi driver for IW61x")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/net/wireless/nxp/nxpwifi/cfg80211.c | 5 +++--
 drivers/net/wireless/nxp/nxpwifi/main.c     | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/nxp/nxpwifi/cfg80211.c b/drivers/net/wireless/nxp/nxpwifi/cfg80211.c
index 5cc8cdf594d3e..461e3e1947438 100644
--- a/drivers/net/wireless/nxp/nxpwifi/cfg80211.c
+++ b/drivers/net/wireless/nxp/nxpwifi/cfg80211.c
@@ -3729,8 +3729,9 @@ int nxpwifi_init_channel_scan_gap(struct nxpwifi_adapter *adapter)
 	 * additional active scan request for hidden SSIDs on passive channels.
 	 */
 	adapter->num_in_chan_stats = 2 * (n_channels_bg + n_channels_a);
-	adapter->chan_stats = vmalloc(array_size(sizeof(*adapter->chan_stats),
-						 adapter->num_in_chan_stats));
+	adapter->chan_stats = kcalloc(adapter->num_in_chan_stats,
+				      sizeof(*adapter->chan_stats),
+				      GFP_KERNEL);
 
 	if (!adapter->chan_stats)
 		return -ENOMEM;
diff --git a/drivers/net/wireless/nxp/nxpwifi/main.c b/drivers/net/wireless/nxp/nxpwifi/main.c
index b4c63829024a0..c5d078bb45983 100644
--- a/drivers/net/wireless/nxp/nxpwifi/main.c
+++ b/drivers/net/wireless/nxp/nxpwifi/main.c
@@ -644,7 +644,7 @@ static int _nxpwifi_fw_dpc(const struct firmware *firmware, void *context)
 	goto done;
 
 err_add_intf:
-	vfree(adapter->chan_stats);
+	kfree(adapter->chan_stats);
 err_init_chan_scan:
 	wiphy_unregister(adapter->wiphy);
 	wiphy_free(adapter->wiphy);
@@ -1384,7 +1384,7 @@ static void nxpwifi_uninit_sw(struct nxpwifi_adapter *adapter)
 	wiphy_free(adapter->wiphy);
 	adapter->wiphy = NULL;
 
-	vfree(adapter->chan_stats);
+	kfree(adapter->chan_stats);
 	nxpwifi_free_cmd_buffers(adapter);
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH wireless-next 1/3] wifi: nxpwifi: wait for the wakeup timer before the adapter is freed
  2026-08-13  8:23 ` [PATCH wireless-next 1/3] wifi: nxpwifi: wait for the wakeup timer before the adapter is freed Linmao Li
@ 2026-09-01 10:13   ` Jeff Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Chen @ 2026-09-01 10:13 UTC (permalink / raw)
  To: Linmao Li; +Cc: Francesco Dolcini, Johannes Berg, linux-wireless, linux-kernel

On Thu, Aug 13, 2026 at 04:23:13 PM +0800, Linmao Li wrote:
> nxpwifi_adapter_cleanup() stops adapter->wakeup_timer with
> timer_delete(), which does not wait for a running callback.  The
> callback goes on using the adapter: wakeup_timer_fn() sets
> adapter->hw_status, walks the command queues through
> nxpwifi_cancel_all_pending_cmd() and calls adapter->if_ops.card_reset().
> Both paths that reach nxpwifi_adapter_cleanup() free the adapter right
> afterwards, through nxpwifi_free_adapter() in nxpwifi_remove_card() and
> in the nxpwifi_add_card() error unwind.
> 
> Use timer_delete_sync() so the callback has finished before the adapter
> is released.
> 

Reviewed-by: Jeff Chen <jeff.chen_1@nxp.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH wireless-next 2/3] wifi: nxpwifi: free the aggregation buffer when the RA list disappears
  2026-08-13  8:23 ` [PATCH wireless-next 2/3] wifi: nxpwifi: free the aggregation buffer when the RA list disappears Linmao Li
@ 2026-09-08 10:21   ` Jeff Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Chen @ 2026-09-08 10:21 UTC (permalink / raw)
  To: Linmao Li; +Cc: Francesco Dolcini, Johannes Berg, linux-wireless, linux-kernel

On Thu, Aug 13, 2026 at 04:23:14 PM +0800, Linmao Li wrote:
> nxpwifi_11n_aggregate_pkt() drops ra_list_spinlock while it copies each
> subframe, so it rechecks the RA list after taking the lock again.  The
> check inside the aggregation loop returns without releasing the
> skb_aggr it has been filling, leaking one tx_buf_size buffer along with
> the subframes already aggregated into it.
> 

Reviewed-by: Jeff Chen <jeff.chen_1@nxp.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH wireless-next 3/3] wifi: nxpwifi: zero the channel statistics array
  2026-08-13  8:23 ` [PATCH wireless-next 3/3] wifi: nxpwifi: zero the channel statistics array Linmao Li
@ 2026-09-08 10:31   ` Jeff Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Chen @ 2026-09-08 10:31 UTC (permalink / raw)
  To: Linmao Li; +Cc: Francesco Dolcini, Johannes Berg, linux-wireless, linux-kernel

On Thu, Aug 13, 2026 at 04:23:15 PM +0800, Linmao Li wrote:
> adapter->chan_stats comes from vmalloc(), which does not clear the
> memory, and nxpwifi_cfg80211_dump_survey() reads it as soon as user
> space asks for survey data.  For the entries no scan has filled in, a
> non-zero cca_scan_dur passes the validity check and stale bytes are
> reported to user space as noise, time and time_busy.
> 
> Use kcalloc() instead.  The array holds two entries per supported
> channel, small enough not to need vmalloc(), and the two callers change
> to kfree() accordingly.
 
Reviewed-by: Jeff Chen <jeff.chen_1@nxp.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-09-08 10:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13  8:23 [PATCH wireless-next 0/3] wifi: nxpwifi: three fixes mwifiex already made Linmao Li
2026-08-13  8:23 ` [PATCH wireless-next 1/3] wifi: nxpwifi: wait for the wakeup timer before the adapter is freed Linmao Li
2026-09-01 10:13   ` Jeff Chen
2026-08-13  8:23 ` [PATCH wireless-next 2/3] wifi: nxpwifi: free the aggregation buffer when the RA list disappears Linmao Li
2026-09-08 10:21   ` Jeff Chen
2026-08-13  8:23 ` [PATCH wireless-next 3/3] wifi: nxpwifi: zero the channel statistics array Linmao Li
2026-09-08 10:31   ` Jeff Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox