Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH] wifi: ath12k: fix channel list double-free on error paths
@ 2026-07-31  9:38 Linkai Gong
  2026-08-07  1:47 ` Jeff Johnson
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Linkai Gong @ 2026-07-31  9:38 UTC (permalink / raw)
  To: Jeff Johnson
  Cc: Vasanthakumar Thiagarajan, Rameshkumar Sundaram, linux-wireless,
	ath12k, linux-kernel, Linkai Gong

ath12k_mac_setup_channels_rates() frees band channel arrays on failure
but either leaves the pointers non-NULL or clears the wrong band. Later
ath12k_mac_cleanup_unregister() frees the same pointers again.

Clear the correct sbands[].channels pointers after kfree(), including
a copy-paste bug that nulled 2 GHz after freeing 6 GHz channels.

Fixes: acc152f9be20 ("wifi: ath12k: combine channel list for split-phy devices in single-wiphy")
Signed-off-by: Linkai Gong <gonglinkai@kylinos.cn>
---
 drivers/net/wireless/ath/ath12k/mac.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index a0928890671a..5468a8d2d5d5 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -14275,6 +14275,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
 					   sizeof(ath12k_6ghz_channels), GFP_KERNEL);
 			if (!channels) {
 				kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
+				ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
 				return -ENOMEM;
 			}
 
@@ -14325,7 +14326,9 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
 					   GFP_KERNEL);
 			if (!channels) {
 				kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
+				ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
 				kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
+				ar->mac.sbands[NL80211_BAND_6GHZ].channels = NULL;
 				return -ENOMEM;
 			}
 
@@ -14365,7 +14368,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
 					kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
 					ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
 					kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
-					ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
+					ar->mac.sbands[NL80211_BAND_6GHZ].channels = NULL;
 					kfree(channels);
 					band->channels = NULL;
 					return ret;
-- 
2.25.1


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

* Re: [PATCH] wifi: ath12k: fix channel list double-free on error paths
  2026-07-31  9:38 [PATCH] wifi: ath12k: fix channel list double-free on error paths Linkai Gong
@ 2026-08-07  1:47 ` Jeff Johnson
  2026-08-07  2:56   ` Linkai Gong
  2026-08-10  9:37 ` Baochen Qiang
  2026-08-10 10:14 ` Rameshkumar Sundaram
  2 siblings, 1 reply; 8+ messages in thread
From: Jeff Johnson @ 2026-08-07  1:47 UTC (permalink / raw)
  To: Linkai Gong, Jeff Johnson
  Cc: Vasanthakumar Thiagarajan, Rameshkumar Sundaram, linux-wireless,
	ath12k, linux-kernel

On 7/31/2026 2:38 AM, Linkai Gong wrote:
> ath12k_mac_setup_channels_rates() frees band channel arrays on failure
> but either leaves the pointers non-NULL or clears the wrong band. Later
> ath12k_mac_cleanup_unregister() frees the same pointers again.
> 
> Clear the correct sbands[].channels pointers after kfree(), including
> a copy-paste bug that nulled 2 GHz after freeing 6 GHz channels.

https://wireless.docs.kernel.org/en/latest/en/users/drivers/ath12k/submittingpatches.html#tested-on-tag
Tested-on: tag missing. How was this tested?

> 
> Fixes: acc152f9be20 ("wifi: ath12k: combine channel list for split-phy devices in single-wiphy")
> Signed-off-by: Linkai Gong <gonglinkai@kylinos.cn>
> ---
>  drivers/net/wireless/ath/ath12k/mac.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index a0928890671a..5468a8d2d5d5 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -14275,6 +14275,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
>  					   sizeof(ath12k_6ghz_channels), GFP_KERNEL);
>  			if (!channels) {
>  				kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
> +				ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
>  				return -ENOMEM;
>  			}
>  
> @@ -14325,7 +14326,9 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
>  					   GFP_KERNEL);
>  			if (!channels) {
>  				kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
> +				ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
>  				kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
> +				ar->mac.sbands[NL80211_BAND_6GHZ].channels = NULL;
>  				return -ENOMEM;
>  			}
>  
> @@ -14365,7 +14368,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
>  					kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
>  					ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
>  					kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
> -					ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
> +					ar->mac.sbands[NL80211_BAND_6GHZ].channels = NULL;
>  					kfree(channels);
>  					band->channels = NULL;
>  					return ret;


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

* Re: [PATCH] wifi: ath12k: fix channel list double-free on error paths
  2026-08-07  1:47 ` Jeff Johnson
@ 2026-08-07  2:56   ` Linkai Gong
  2026-08-07 15:48     ` Jeff Johnson
  0 siblings, 1 reply; 8+ messages in thread
From: Linkai Gong @ 2026-08-07  2:56 UTC (permalink / raw)
  To: Jeff Johnson
  Cc: Jeff Johnson, Vasanthakumar Thiagarajan, Rameshkumar Sundaram,
	linux-wireless, ath12k, linux-kernel

On Thu, Aug 06, 2026 at 18:47:38 -0700, Jeff Johnson wrote:
> https://wireless.docs.kernel.org/en/latest/en/users/drivers/ath12k/submittingpatches.html#tested-on-tag
> Tested-on: tag missing. How was this tested?

Thanks for the review.

This issue was found by code review of the error paths in
ath12k_mac_setup_channels_rates(). I do not have QCN9274/WCN7850
hardware, so I could not provide a Tested-on tag or reproduce the
ENOMEM double-free on a device.

The bug is: after kfree() of sbands[].channels on failure, the
pointer is left non-NULL (or the wrong band is cleared), and
ath12k_mac_cleanup_unregister() frees it again.

I understand Tested-on is required for ath12k. I will follow up
with a v2 including Tested-on once I can test on QCN9274/WCN7850
hardware. Meanwhile, any Tested-by from someone with the hardware
would be appreciated.

Thanks,
Linkai

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

* Re: [PATCH] wifi: ath12k: fix channel list double-free on error paths
  2026-08-07  2:56   ` Linkai Gong
@ 2026-08-07 15:48     ` Jeff Johnson
  0 siblings, 0 replies; 8+ messages in thread
From: Jeff Johnson @ 2026-08-07 15:48 UTC (permalink / raw)
  To: Linkai Gong
  Cc: Jeff Johnson, Vasanthakumar Thiagarajan, Rameshkumar Sundaram,
	linux-wireless, ath12k, linux-kernel

On 8/6/2026 7:56 PM, Linkai Gong wrote:
> On Thu, Aug 06, 2026 at 18:47:38 -0700, Jeff Johnson wrote:
>> https://wireless.docs.kernel.org/en/latest/en/users/drivers/ath12k/submittingpatches.html#tested-on-tag
>> Tested-on: tag missing. How was this tested?
> 
> Thanks for the review.
> 
> This issue was found by code review of the error paths in
> ath12k_mac_setup_channels_rates(). I do not have QCN9274/WCN7850
> hardware, so I could not provide a Tested-on tag or reproduce the
> ENOMEM double-free on a device.
> 
> The bug is: after kfree() of sbands[].channels on failure, the
> pointer is left non-NULL (or the wrong band is cleared), and
> ath12k_mac_cleanup_unregister() frees it again.
> 
> I understand Tested-on is required for ath12k. I will follow up
> with a v2 including Tested-on once I can test on QCN9274/WCN7850
> hardware. Meanwhile, any Tested-by from someone with the hardware
> would be appreciated.
It is OK to specify the patch was compiled tested only. I just need that as
part of the commit text so that the level of testing is documented.

The ath10k documentation was clear about this:
For trivial patches it’s ok to skip Tested-on tag but then it should say
“Compile tested only”.

Not sure why that was dropped as part of the ath11k and ath12k documentation
(that predates me becoming a maintainer).

/jeff

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

* Re: [PATCH] wifi: ath12k: fix channel list double-free on error paths
  2026-07-31  9:38 [PATCH] wifi: ath12k: fix channel list double-free on error paths Linkai Gong
  2026-08-07  1:47 ` Jeff Johnson
@ 2026-08-10  9:37 ` Baochen Qiang
  2026-08-11  2:05   ` Linkai Gong
  2026-08-10 10:14 ` Rameshkumar Sundaram
  2 siblings, 1 reply; 8+ messages in thread
From: Baochen Qiang @ 2026-08-10  9:37 UTC (permalink / raw)
  To: Linkai Gong, Jeff Johnson
  Cc: Vasanthakumar Thiagarajan, Rameshkumar Sundaram, linux-wireless,
	ath12k, linux-kernel



On 7/31/2026 5:38 PM, Linkai Gong wrote:
> ath12k_mac_setup_channels_rates() frees band channel arrays on failure
> but either leaves the pointers non-NULL or clears the wrong band. Later
> ath12k_mac_cleanup_unregister() frees the same pointers again.

The radio that frees its sbands[].channels inside setup_channels_rates() is precisely the
one excluded from cleanup_unregister() — the loop is for (j = 0; j < i; j++) and the
failing radio is index i. The earlier radios (0..i-1) all fully succeeded, so their
channel arrays are valid and freed exactly once. So this is actually a dangling pointer
issue, not a reachable double-free — nothing frees the failing radio's pointers a second
time. Please reword the message to something like "error paths leave dangling pointers + a
copy-paste typo; harden by consistently nulling after kfree()."

> 
> Clear the correct sbands[].channels pointers after kfree(), including
> a copy-paste bug that nulled 2 GHz after freeing 6 GHz channels.
> 
> Fixes: acc152f9be20 ("wifi: ath12k: combine channel list for split-phy devices in single-wiphy")
> Signed-off-by: Linkai Gong <gonglinkai@kylinos.cn>
> ---
>  drivers/net/wireless/ath/ath12k/mac.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index a0928890671a..5468a8d2d5d5 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -14275,6 +14275,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
>  					   sizeof(ath12k_6ghz_channels), GFP_KERNEL);
>  			if (!channels) {
>  				kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
> +				ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
>  				return -ENOMEM;
>  			}
>  
> @@ -14325,7 +14326,9 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
>  					   GFP_KERNEL);
>  			if (!channels) {
>  				kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
> +				ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
>  				kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
> +				ar->mac.sbands[NL80211_BAND_6GHZ].channels = NULL;
>  				return -ENOMEM;
>  			}
>  
> @@ -14365,7 +14368,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
>  					kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
>  					ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
>  					kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
> -					ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
> +					ar->mac.sbands[NL80211_BAND_6GHZ].channels = NULL;
>  					kfree(channels);
>  					band->channels = NULL;
>  					return ret;

actual code change looks good


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

* Re: [PATCH] wifi: ath12k: fix channel list double-free on error paths
  2026-07-31  9:38 [PATCH] wifi: ath12k: fix channel list double-free on error paths Linkai Gong
  2026-08-07  1:47 ` Jeff Johnson
  2026-08-10  9:37 ` Baochen Qiang
@ 2026-08-10 10:14 ` Rameshkumar Sundaram
  2026-08-11  2:06   ` Linkai Gong
  2 siblings, 1 reply; 8+ messages in thread
From: Rameshkumar Sundaram @ 2026-08-10 10:14 UTC (permalink / raw)
  To: Linkai Gong, Jeff Johnson
  Cc: Vasanthakumar Thiagarajan, linux-wireless, ath12k, linux-kernel

On 7/31/2026 3:08 PM, Linkai Gong wrote:
> ath12k_mac_setup_channels_rates() frees band channel arrays on failure
> but either leaves the pointers non-NULL or clears the wrong band. Later
> ath12k_mac_cleanup_unregister() frees the same pointers again.
> 
> Clear the correct sbands[].channels pointers after kfree(), including
> a copy-paste bug that nulled 2 GHz after freeing 6 GHz channels.
> 
> Fixes: acc152f9be20 ("wifi: ath12k: combine channel list for split-phy devices in single-wiphy")
> Signed-off-by: Linkai Gong <gonglinkai@kylinos.cn>
> ---
>   drivers/net/wireless/ath/ath12k/mac.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index a0928890671a..5468a8d2d5d5 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -14275,6 +14275,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
>   					   sizeof(ath12k_6ghz_channels), GFP_KERNEL);
>   			if (!channels) {
>   				kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
> +				ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
>   				return -ENOMEM;
>   			}
>   
> @@ -14325,7 +14326,9 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
>   					   GFP_KERNEL);
>   			if (!channels) {
>   				kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
> +				ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
>   				kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
> +				ar->mac.sbands[NL80211_BAND_6GHZ].channels = NULL;
>   				return -ENOMEM;
>   			}
> 


Above two hunks seem to fix older allocation-failure paths which
already existed before acc152f9be20. In current history those lines 
blame back to d889913205cf ("wifi: ath12k: driver for Qualcomm IEEE 
802.11ax devices").

So this probably should either be split into two fixes, or the commit
message/Fixes tag should be adjusted.

But unless there is another reachable path where these stale pointers 
are later freed or dereferenced, do we really need a Fixes tag here?




> @@ -14365,7 +14368,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
>   					kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
>   					ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
>   					kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
> -					ar->mac.sbands[NL80211_BAND_2GHZ].channels = NULL;
> +					ar->mac.sbands[NL80211_BAND_6GHZ].channels = NULL;
>   					kfree(channels);
>   					band->channels = NULL;
>   					return ret;



--
Ramesh

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

* Re: [PATCH] wifi: ath12k: fix channel list double-free on error paths
  2026-08-10  9:37 ` Baochen Qiang
@ 2026-08-11  2:05   ` Linkai Gong
  0 siblings, 0 replies; 8+ messages in thread
From: Linkai Gong @ 2026-08-11  2:05 UTC (permalink / raw)
  To: baochen.qiang
  Cc: jjohnson, vasanthakumar.thiagarajan, rameshkumar.sundaram,
	linux-wireless, ath12k, linux-kernel

On Mon, Aug 10, 2026 at 03:07:25PM +0530, Baochen Qiang wrote:
> So this is actually a dangling pointer
> issue, not a reachable double-free — nothing frees the failing radio's pointers a second
> time. Please reword the message to something like "error paths leave dangling pointers + a
> copy-paste typo; harden by consistently nulling after kfree()."

Thanks. You are right, I misread the cleanup loop.

Will reword the commit message in v2. Code change stays the same.

Thanks,
Linkai

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

* Re: [PATCH] wifi: ath12k: fix channel list double-free on error paths
  2026-08-10 10:14 ` Rameshkumar Sundaram
@ 2026-08-11  2:06   ` Linkai Gong
  0 siblings, 0 replies; 8+ messages in thread
From: Linkai Gong @ 2026-08-11  2:06 UTC (permalink / raw)
  To: rameshkumar.sundaram
  Cc: jjohnson, baochen.qiang, vasanthakumar.thiagarajan,
	linux-wireless, ath12k, linux-kernel

On Mon, Aug 10, 2026 at 03:44:54PM +0530, Rameshkumar Sundaram wrote:
> So this probably should either be split into two fixes, or the commit
> message/Fixes tag should be adjusted.
>
> But unless there is another reachable path where these stale pointers
> are later freed or dereferenced, do we really need a Fixes tag here?

Thanks. Agree on both points , the first two hunks predate
acc152f9be20 , and with Baochen's clarification there is no reachable
second free, so a Fixes tag does not make sense.

v2 will drop Fixes and keep this as one small hardening patch.

Thanks,
Linkai

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

end of thread, other threads:[~2026-08-11  2:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-31  9:38 [PATCH] wifi: ath12k: fix channel list double-free on error paths Linkai Gong
2026-08-07  1:47 ` Jeff Johnson
2026-08-07  2:56   ` Linkai Gong
2026-08-07 15:48     ` Jeff Johnson
2026-08-10  9:37 ` Baochen Qiang
2026-08-11  2:05   ` Linkai Gong
2026-08-10 10:14 ` Rameshkumar Sundaram
2026-08-11  2:06   ` Linkai Gong

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