[PATCH wireless-next v3 0/3] misc chandef cleanups

[PATCH wireless-next v3 0/3] misc chandef cleanups

[Lachlan Hodges]

It was discussed in [1] that calling cfg80211_create_chandef() probably
doesn't make the most sense for non-HT based bands (that being S1G and
60GHz currently). Even though it's called fairly frequently, most of
those paths are unrelated or not supported yet. However creating
the default chandef is one. So creating a helper specifically for
creating the default chandef which is band-agnostic makes more
sense. Additionally, insert a WARN into cfg80211_chandef_create() to
catch any misuses by S1G and 60GHz bands.

hwsim tests were run just to double check nothing broke on hostap
tip 11620497a ("EPPKE: Do not start Authenticator state machine on
reassociation") and wireless-next 97492c019da4 ("wifi: mwifiex: drop
redundant device reference").

The 3rd patch adds some simple validation for when we have an S1G
chandef but a non-S1G width to catch any weird corner cases like one
discussed in [2].

[1] https://lore.kernel.org/linux-wireless/6832f8f0b516157452bd9c23b7c7af087d63d425.camel@sipsolutions.net/T/#mdd8f8115f3c8195638568cface3e20ab777f9f33
[2] https://lore.kernel.org/linux-wireless/20260311061800.517849-1-lachlan.hodges@morsemicro.com/

lachlan

Lachlan Hodges (3):
  wifi: mac80211: don't use cfg80211_chandef_create() for default
    chandef
  wifi: cfg80211: restrict cfg80211_chandef_create() to only HT-based
    bands
  wifi: cfg80211: check non-S1G width with S1G chandef

 net/mac80211/main.c | 18 +++++++++++++++---
 net/wireless/chan.c | 12 +++++++++++-
 2 files changed, 26 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH wireless-next v3 1/3] wifi: mac80211: don't use cfg80211_chandef_create() for default chandef

[Lachlan Hodges]

cfg80211_chandef_create() is called universally to create the
default chandef during hw registration, however it only really
makes sense to be used for 2GHz, 5GHz, and 6GHz (and by extension
the 'LC' band) as it relies on the channel type which is only
relevant to those specific bands.

To reduce some confusion, create a generic helper for creating the
default chandef that makes sense for all supported bands.

Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
 net/mac80211/main.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 616f86b1a7e4..ed5d60328041 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1117,6 +1117,19 @@ ieee80211_ifcomb_check(const struct ieee80211_iface_combination *c, int n_comb)
 	return true;
 }
 
+static void ieee80211_create_default_chandef(struct cfg80211_chan_def *chandef,
+					     struct ieee80211_channel *chan)
+{
+	*chandef = (struct cfg80211_chan_def) {
+		.chan = chan,
+		.width = chan->band == NL80211_BAND_S1GHZ ?
+				 NL80211_CHAN_WIDTH_1 :
+				 NL80211_CHAN_WIDTH_20_NOHT,
+		.center_freq1 = chan->center_freq,
+		.freq1_offset = chan->freq_offset,
+	};
+}
+
 int ieee80211_register_hw(struct ieee80211_hw *hw)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
@@ -1260,9 +1273,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 			/* if none found then use the first anyway */
 			if (i == sband->n_channels)
 				i = 0;
-			cfg80211_chandef_create(&dflt_chandef,
-						&sband->channels[i],
-						NL80211_CHAN_NO_HT);
+			ieee80211_create_default_chandef(&dflt_chandef,
+							 &sband->channels[i]);
 			/* init channel we're on */
 			local->monitor_chanreq.oper = dflt_chandef;
 			if (local->emulate_chanctx) {
-- 
2.43.0

[PATCH wireless-next v3 2/3] wifi: cfg80211: restrict cfg80211_chandef_create() to only HT-based bands

[Lachlan Hodges]

cfg80211_chandef_create() should only be used by bands that are
HT-based and the chantype argument makes sense. Insert a WARN such
that it isn't used on 60GHz and S1GHz bands and to catch any
potential existing uses by those bands.

Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
 net/wireless/chan.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 68221b1ab45e..4d2c2b9f1eed 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -29,9 +29,11 @@ void cfg80211_chandef_create(struct cfg80211_chan_def *chandef,
 
 	*chandef = (struct cfg80211_chan_def) {
 		.chan = chan,
-		.freq1_offset = chan->freq_offset,
 	};
 
+	WARN_ON(chan->band == NL80211_BAND_60GHZ ||
+		chan->band == NL80211_BAND_S1GHZ);
+
 	switch (chan_type) {
 	case NL80211_CHAN_NO_HT:
 		chandef->width = NL80211_CHAN_WIDTH_20_NOHT;
-- 
2.43.0

[PATCH wireless-next v3 3/3] wifi: cfg80211: check non-S1G width with S1G chandef

[Lachlan Hodges]

It is not valid to have an S1G chandef with a non-S1G width. Enforce
this during chandef validation.

Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
 net/wireless/chan.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 4d2c2b9f1eed..f80bc5144037 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -353,6 +353,14 @@ bool cfg80211_chandef_valid(const struct cfg80211_chan_def *chandef)
 
 	control_freq = chandef->chan->center_freq;
 
+	if (cfg80211_chandef_is_s1g(chandef) &&
+	    chandef->width != NL80211_CHAN_WIDTH_1 &&
+	    chandef->width != NL80211_CHAN_WIDTH_2 &&
+	    chandef->width != NL80211_CHAN_WIDTH_4 &&
+	    chandef->width != NL80211_CHAN_WIDTH_8 &&
+	    chandef->width != NL80211_CHAN_WIDTH_16)
+		return false;
+
 	switch (chandef->width) {
 	case NL80211_CHAN_WIDTH_5:
 	case NL80211_CHAN_WIDTH_10:
-- 
2.43.0

Re: [PATCH wireless-next v3 3/3] wifi: cfg80211: check non-S1G width with S1G chandef

[Lachlan Hodges]

> +	if (cfg80211_chandef_is_s1g(chandef) &&
> +	    chandef->width != NL80211_CHAN_WIDTH_1 &&
> +	    chandef->width != NL80211_CHAN_WIDTH_2 &&
> +	    chandef->width != NL80211_CHAN_WIDTH_4 &&
> +	    chandef->width != NL80211_CHAN_WIDTH_8 &&
> +	    chandef->width != NL80211_CHAN_WIDTH_16)
> +		return false;
> +

Admittedly this makes this already noisy functiona fair bit noiser. An
option was to do the following:

	switch (chandef->width) {
	case NL80211_CHAN_WIDTH_5:
	case NL80211_CHAN_WIDTH_10:
	case NL80211_CHAN_WIDTH_20:
	case NL80211_CHAN_WIDTH_20_NOHT:
		if (ieee80211_chandef_to_khz(chandef) !=
		    ieee80211_channel_to_khz(chandef->chan))
			return false;
		if (chandef->center_freq2)
			return false;
+		if (cfg80211_chandef_is_s1g(chandef))
+			return false;
		break;

Since that would cover all the cases where a chandef is initialised,
but not some weird edge case where maybe an S1G chandef somehow got a
80MHz width or something but I guess better to cover all cases.

lachlan

Re: [PATCH wireless-next v3 0/3] misc chandef cleanups

[Lachlan Hodges]

> Lachlan Hodges (3):
>   wifi: mac80211: don't use cfg80211_chandef_create() for default
>     chandef
>   wifi: cfg80211: restrict cfg80211_chandef_create() to only HT-based
>     bands
>   wifi: cfg80211: check non-S1G width with S1G chandef

Ah something I forgot to mention is I put the mac80211 patch first,
otherwise maybe during some bisect or something it would break if
we changed the cfg80211_create_chandef() without modifying the
creation of the default one first.

lachlan

[syzbot ci] Re: misc chandef cleanups

[syzbot ci]

syzbot ci has tested the following series

[v3] misc chandef cleanups
https://lore.kernel.org/all/20260312045804.362974-1-lachlan.hodges@morsemicro.com
* [PATCH wireless-next v3 1/3] wifi: mac80211: don't use cfg80211_chandef_create() for default chandef
* [PATCH wireless-next v3 2/3] wifi: cfg80211: restrict cfg80211_chandef_create() to only HT-based bands
* [PATCH wireless-next v3 3/3] wifi: cfg80211: check non-S1G width with S1G chandef

and found the following issue:
WARNING in cfg80211_chandef_create

Full report is available here:
https://ci.syzbot.org/series/ce6fc7d6-d8d4-4d00-a746-db78cba13e47

***

WARNING in cfg80211_chandef_create

tree:      torvalds
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux
base:      80234b5ab240f52fa45d201e899e207b9265ef91
arch:      amd64
compiler:  Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
config:    https://ci.syzbot.org/builds/0a18d006-45de-4bb2-a6e2-2cbb788bd2f5/config
C repro:   https://ci.syzbot.org/findings/c279480a-56a9-4a59-b533-0033d65eca62/c_repro
syz repro: https://ci.syzbot.org/findings/c279480a-56a9-4a59-b533-0033d65eca62/syz_repro

netlink: 8 bytes leftover after parsing attributes in process `syz.0.17'.
------------[ cut here ]------------
chan->band == NL80211_BAND_60GHZ || chan->band == NL80211_BAND_S1GHZ
WARNING: net/wireless/chan.c:35 at cfg80211_chandef_create+0x99/0x3d0 net/wireless/chan.c:34, CPU#1: syz.0.17/5951
Modules linked in:
CPU: 1 UID: 0 PID: 5951 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:cfg80211_chandef_create+0x99/0x3d0 net/wireless/chan.c:34
Code: 8b 26 4c 89 e7 48 c7 c6 40 d6 e3 8f e8 a0 7d bd f6 49 83 fc 04 74 0d 41 83 fc 02 75 12 e8 0f 78 bd f6 eb 05 e8 08 78 bd f6 90 <0f> 0b 90 eb 05 e8 fd 77 bd f6 89 ef 48 c7 c6 60 d6 e3 8f e8 6f 7d
RSP: 0018:ffffc900054fef78 EFLAGS: 00010293
RAX: ffffffff8b0825a8 RBX: ffffc900054ff0e0 RCX: ffff888112b5ba80
RDX: 0000000000000000 RSI: ffffffff8fe3d640 RDI: 0000000000000004
RBP: 0000000000000002 R08: ffff888112b5ba80 R09: 0000000000000002
R10: 0000000000000004 R11: 0000000000000000 R12: 0000000000000004
R13: dffffc0000000000 R14: ffff888111c65998 R15: ffffc900054ff0e8
FS:  000055558dcaa500(0000) GS:ffff8882a9463000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f14d1588095 CR3: 000000010668a000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 _nl80211_parse_chandef+0x438/0x1160 net/wireless/nl80211.c:3616
 __nl80211_set_channel+0x1fe/0x850 net/wireless/nl80211.c:3736
 nl80211_set_wiphy+0x116b/0x2fa0 net/wireless/nl80211.c:-1
 genl_family_rcv_msg_doit+0x22a/0x330 net/netlink/genetlink.c:1114
 genl_family_rcv_msg net/netlink/genetlink.c:1194 [inline]
 genl_rcv_msg+0x61c/0x7a0 net/netlink/genetlink.c:1209
 netlink_rcv_skb+0x232/0x4b0 net/netlink/af_netlink.c:2550
 genl_rcv+0x28/0x40 net/netlink/genetlink.c:1218
 netlink_unicast_kernel net/netlink/af_netlink.c:1318 [inline]
 netlink_unicast+0x80f/0x9b0 net/netlink/af_netlink.c:1344
 netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1894
 sock_sendmsg_nosec net/socket.c:727 [inline]
 __sock_sendmsg net/socket.c:742 [inline]
 ____sys_sendmsg+0x972/0x9f0 net/socket.c:2592
 ___sys_sendmsg+0x2a5/0x360 net/socket.c:2646
 __sys_sendmsg net/socket.c:2678 [inline]
 __do_sys_sendmsg net/socket.c:2683 [inline]
 __se_sys_sendmsg net/socket.c:2681 [inline]
 __x64_sys_sendmsg+0x1bd/0x2a0 net/socket.c:2681
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x14d/0xf80 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f0ea239c799
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe7ad20a38 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f0ea2615fa0 RCX: 00007f0ea239c799
RDX: 0000000000000000 RSI: 0000200000000040 RDI: 0000000000000003
RBP: 00007f0ea2432bd9 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f0ea2615fac R14: 00007f0ea2615fa0 R15: 00007f0ea2615fa0
 </TASK>


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

Re: [syzbot ci] Re: misc chandef cleanups

[Johannes Berg]

On Fri, 2026-03-13 at 01:45 -0700, syzbot ci wrote:
> syzbot ci has tested the following series
> 
> [v3] misc chandef cleanups
> https://lore.kernel.org/all/20260312045804.362974-1-lachlan.hodges@morsemicro.com
> * [PATCH wireless-next v3 1/3] wifi: mac80211: don't use cfg80211_chandef_create() for default chandef
> * [PATCH wireless-next v3 2/3] wifi: cfg80211: restrict cfg80211_chandef_create() to only HT-based bands
> * [PATCH wireless-next v3 3/3] wifi: cfg80211: check non-S1G width with S1G chandef
> 
> and found the following issue:
> WARNING in cfg80211_chandef_create
> 
> Full report is available here:
> https://ci.syzbot.org/series/ce6fc7d6-d8d4-4d00-a746-db78cba13e47
> 
> ***
> 
> WARNING in cfg80211_chandef_create

D'oh, just after I apply it.

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3e867930e253..7314312ec567 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3634,8 +3634,6 @@ static int _nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
 		case NL80211_CHAN_HT20:
 		case NL80211_CHAN_HT40PLUS:
 		case NL80211_CHAN_HT40MINUS:
-			cfg80211_chandef_create(chandef, chandef->chan,
-						chantype);
 			/* user input for center_freq is incorrect */
 			if (attrs[NL80211_ATTR_CENTER_FREQ1] &&
 			    chandef->center_freq1 != nla_get_u32(attrs[NL80211_ATTR_CENTER_FREQ1])) {
@@ -3652,6 +3650,11 @@ static int _nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
 						    "center frequency 2 can't be used");
 				return -EINVAL;
 			}
+			if (chandef->chan->band == NL80211_BAND_60GHZ ||
+			    chandef->chan->band == NL80211_BAND_S1GHZ)
+				return -EINVAL;
+			cfg80211_chandef_create(chandef, chandef->chan,
+						chantype);
 			break;
 		default:
 			NL_SET_ERR_MSG_ATTR(extack,


I think?

johannes

Re: [syzbot ci] Re: misc chandef cleanups

[Lachlan Hodges]

On Fri, Mar 13, 2026 at 10:05:15AM +0100, Johannes Berg wrote:
> On Fri, 2026-03-13 at 01:45 -0700, syzbot ci wrote:
> > syzbot ci has tested the following series
> > 
> > [v3] misc chandef cleanups
> > https://lore.kernel.org/all/20260312045804.362974-1-lachlan.hodges@morsemicro.com
> > * [PATCH wireless-next v3 1/3] wifi: mac80211: don't use cfg80211_chandef_create() for default chandef
> > * [PATCH wireless-next v3 2/3] wifi: cfg80211: restrict cfg80211_chandef_create() to only HT-based bands
> > * [PATCH wireless-next v3 3/3] wifi: cfg80211: check non-S1G width with S1G chandef
> > 
> > and found the following issue:
> > WARNING in cfg80211_chandef_create
> > 
> > Full report is available here:
> > https://ci.syzbot.org/series/ce6fc7d6-d8d4-4d00-a746-db78cba13e47
> > 
> > ***
> > 
> > WARNING in cfg80211_chandef_create
> 
> D'oh, just after I apply it.

That didn't take long ^.^

> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 3e867930e253..7314312ec567 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -3634,8 +3634,6 @@ static int _nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
>  		case NL80211_CHAN_HT20:
>  		case NL80211_CHAN_HT40PLUS:
>  		case NL80211_CHAN_HT40MINUS:
> -			cfg80211_chandef_create(chandef, chandef->chan,
> -						chantype);
>  			/* user input for center_freq is incorrect */
>  			if (attrs[NL80211_ATTR_CENTER_FREQ1] &&
>  			    chandef->center_freq1 != nla_get_u32(attrs[NL80211_ATTR_CENTER_FREQ1])) {
> @@ -3652,6 +3650,11 @@ static int _nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
>  						    "center frequency 2 can't be used");
>  				return -EINVAL;
>  			}
> +			if (chandef->chan->band == NL80211_BAND_60GHZ ||
> +			    chandef->chan->band == NL80211_BAND_S1GHZ)
> +				return -EINVAL;
> +			cfg80211_chandef_create(chandef, chandef->chan,
> +						chantype);
>  			break;
>  		default:
>  			NL_SET_ERR_MSG_ATTR(extack,
> 
> 
> I think?

I'm probably misunderstanding - but cfg80211_chandef_create() modifies
chandef->center_freq1 if you have a HT40+/- chantype wouldn't you
wanna do that before you validate against the CENTER_FREQ1 attribute?
Since in the generic init code above it sets cf1 to the control freq?

[...]
chandef->center_freq1 = KHZ_TO_MHZ(control_freq);
[...]

where it wouldn't match for HT40-/+ since im guessing the CF1 sent
down should be what it would be _after_ being set by
cfg80211_create_chandef() based on the chantype? Or am i missing
something?

lachlan

Re: [syzbot ci] Re: misc chandef cleanups

[Johannes Berg]

On Fri, 2026-03-13 at 20:48 +1100, Lachlan Hodges wrote:
> 
> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> > index 3e867930e253..7314312ec567 100644
> > --- a/net/wireless/nl80211.c
> > +++ b/net/wireless/nl80211.c
> > @@ -3634,8 +3634,6 @@ static int _nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
> >  		case NL80211_CHAN_HT20:
> >  		case NL80211_CHAN_HT40PLUS:
> >  		case NL80211_CHAN_HT40MINUS:
> > -			cfg80211_chandef_create(chandef, chandef->chan,
> > -						chantype);
> >  			/* user input for center_freq is incorrect */
> >  			if (attrs[NL80211_ATTR_CENTER_FREQ1] &&
> >  			    chandef->center_freq1 != nla_get_u32(attrs[NL80211_ATTR_CENTER_FREQ1])) {
> > @@ -3652,6 +3650,11 @@ static int _nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
> >  						    "center frequency 2 can't be used");
> >  				return -EINVAL;
> >  			}
> > +			if (chandef->chan->band == NL80211_BAND_60GHZ ||
> > +			    chandef->chan->band == NL80211_BAND_S1GHZ)
> > +				return -EINVAL;
> > +			cfg80211_chandef_create(chandef, chandef->chan,
> > +						chantype);
> >  			break;
> >  		default:
> >  			NL_SET_ERR_MSG_ATTR(extack,
> > 
> > 
> > I think?
> 
> I'm probably misunderstanding - but cfg80211_chandef_create() modifies
> chandef->center_freq1 if you have a HT40+/- chantype wouldn't you
> wanna do that before you validate against the CENTER_FREQ1 attribute?

Oh, yeah, oops.

> Since in the generic init code above it sets cf1 to the control freq?
> 
> [...]
> chandef->center_freq1 = KHZ_TO_MHZ(control_freq);
> [...]
> 
> where it wouldn't match for HT40-/+ since im guessing the CF1 sent
> down should be what it would be _after_ being set by
> cfg80211_create_chandef() based on the chantype? Or am i missing
> something?

No, I just didn't think about it. I moved it because I thought I'd do
this differently, but this validation can just come first anyway.

johannes