Executing command NL80211_CMD_GET_WIPHY and parsing it's output natively without libnl shows following attributes as part of the nl80211 generated netlink message (part 16):
GetWiphy: Type: 1 GetWiphy: Type: 2 GetWiphy: Type: 46 GetWiphy: Type: 33074 <-- wrong enum value, above MAX also .. GetWiphy: Type: 316
Switching to nla_nest_start_noflag() which ommits the NLA_F_NESTED flag (like most other similar functions do) fixes this:
GetWiphy: Type: 1 GetWiphy: Type: 2 GetWiphy: Type: 46 GetWiphy: Type: 306 <-- correct enum value GetWiphy: Type: 316
Fixes: dc1e3cb8da8b ("nl80211: MBSSID and EMA support in AP mode") Signed-off-by: Koen Vandeputte koen.vandeputte@citymesh.com Cc: Johannes Berg johannes@sipsolutions.net Cc: John Crispin john@phrozen.org Cc: Aloka Dixit alokad@codeaurora.org Cc: stable@vger.kernel.org # 5.16 --- net/wireless/nl80211.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 0da2e6a2a7ea..4d4860f9428d 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -2359,7 +2359,7 @@ static int nl80211_put_mbssid_support(struct wiphy *wiphy, struct sk_buff *msg) if (!wiphy->mbssid_max_interfaces) return 0;
- config = nla_nest_start(msg, NL80211_ATTR_MBSSID_CONFIG); + config = nla_nest_start_noflag(msg, NL80211_ATTR_MBSSID_CONFIG); if (!config) return -ENOBUFS;
On Wed, 2023-07-12 at 10:38 +0200, Koen Vandeputte wrote:
Executing command NL80211_CMD_GET_WIPHY and parsing it's output natively without libnl shows following attributes as part of the nl80211 generated netlink message (part 16):
GetWiphy: Type: 1 GetWiphy: Type: 2 GetWiphy: Type: 46 GetWiphy: Type: 33074 <-- wrong enum value, above MAX also ..
That's not wrong, that's just NLA_F_NESTED | NL80211_ATTR_MBSSID_CONFIG, since it *is* in fact a nested attribute.
Switching to nla_nest_start_noflag() which ommits the NLA_F_NESTED flag (like most other similar functions do) fixes this:
GetWiphy: Type: 1 GetWiphy: Type: 2 GetWiphy: Type: 46 GetWiphy: Type: 306 <-- correct enum value GetWiphy: Type: 316
Let's say it _changes_ it, but it doesn't _fix_ it, since it's not broken.
Using nla_nest_start_noflag() is a legacy thing, it shouldn't be done any more. You need to update your userspace, I'm not applying this patch.
johannes
On Wed, Jul 12, 2023 at 10:44 AM Johannes Berg johannes@sipsolutions.net wrote:
On Wed, 2023-07-12 at 10:38 +0200, Koen Vandeputte wrote:
Executing command NL80211_CMD_GET_WIPHY and parsing it's output natively without libnl shows following attributes as part of the nl80211 generated netlink message (part 16):
GetWiphy: Type: 1 GetWiphy: Type: 2 GetWiphy: Type: 46 GetWiphy: Type: 33074 <-- wrong enum value, above MAX also ..
That's not wrong, that's just NLA_F_NESTED | NL80211_ATTR_MBSSID_CONFIG, since it *is* in fact a nested attribute.
ahha! so one should check on each received attribute if this flag is set?
Switching to nla_nest_start_noflag() which ommits the NLA_F_NESTED flag (like most other similar functions do) fixes this:
GetWiphy: Type: 1 GetWiphy: Type: 2 GetWiphy: Type: 46 GetWiphy: Type: 306 <-- correct enum value GetWiphy: Type: 316
Let's say it _changes_ it, but it doesn't _fix_ it, since it's not broken.
noted
Using nla_nest_start_noflag() is a legacy thing, it shouldn't be done any more. You need to update your userspace, I'm not applying this patch.
Sure, no problem. I guess all the ones with the noflag so far are using it to avoid breaking legacy stuff?
johannes
Thanks for the quick review. Highly appreciated!
On Wed, 2023-07-12 at 11:05 +0200, Koen Vandeputte wrote:
On Wed, Jul 12, 2023 at 10:44 AM Johannes Berg johannes@sipsolutions.net wrote:
On Wed, 2023-07-12 at 10:38 +0200, Koen Vandeputte wrote:
Executing command NL80211_CMD_GET_WIPHY and parsing it's output natively without libnl shows following attributes as part of the nl80211 generated netlink message (part 16):
GetWiphy: Type: 1 GetWiphy: Type: 2 GetWiphy: Type: 46 GetWiphy: Type: 33074 <-- wrong enum value, above MAX also ..
That's not wrong, that's just NLA_F_NESTED | NL80211_ATTR_MBSSID_CONFIG, since it *is* in fact a nested attribute.
ahha! so one should check on each received attribute if this flag is set?
Yeah, that indicates that it's nested, without having the out-of-band information that it should be nested.
There's also NLA_F_NET_BYTEORDER, so we really only have 14 bits for the type, so you should probably use NLA_TYPE_MASK.
I guess all the ones with the noflag so far are using it to avoid breaking legacy stuff?
Right. When we realized that nla_put_nested() didn't actually put the nested flag it was kind of too late - although a lot of userspace probably would have handled it correctly (if it uses libnl, etc.) But still, we converted it all to nla_put_nested_noflag(), but we really shouldn't be adding new code that calls nla_put_nested_noflag() since by definition new code is new attributes, and then userspace can be updated.
Unless you're saying this somehow breaks old userspace that doesn't even understand NL80211_ATTR_MBSSID_CONFIG yet? But that seems unlikely, you have to be prepared to see attributes bigger than what you expected and skip them?
johannes
On Wed, Jul 12, 2023 at 12:34 PM Johannes Berg johannes@sipsolutions.net wrote:
On Wed, 2023-07-12 at 11:05 +0200, Koen Vandeputte wrote:
On Wed, Jul 12, 2023 at 10:44 AM Johannes Berg johannes@sipsolutions.net wrote:
On Wed, 2023-07-12 at 10:38 +0200, Koen Vandeputte wrote:
Executing command NL80211_CMD_GET_WIPHY and parsing it's output natively without libnl shows following attributes as part of the nl80211 generated netlink message (part 16):
GetWiphy: Type: 1 GetWiphy: Type: 2 GetWiphy: Type: 46 GetWiphy: Type: 33074 <-- wrong enum value, above MAX also ..
That's not wrong, that's just NLA_F_NESTED | NL80211_ATTR_MBSSID_CONFIG, since it *is* in fact a nested attribute.
ahha! so one should check on each received attribute if this flag is set?
Yeah, that indicates that it's nested, without having the out-of-band information that it should be nested.
There's also NLA_F_NET_BYTEORDER, so we really only have 14 bits for the type, so you should probably use NLA_TYPE_MASK.
I guess all the ones with the noflag so far are using it to avoid breaking legacy stuff?
Right. When we realized that nla_put_nested() didn't actually put the nested flag it was kind of too late - although a lot of userspace probably would have handled it correctly (if it uses libnl, etc.) But still, we converted it all to nla_put_nested_noflag(), but we really shouldn't be adding new code that calls nla_put_nested_noflag() since by definition new code is new attributes, and then userspace can be updated.
Unless you're saying this somehow breaks old userspace that doesn't even understand NL80211_ATTR_MBSSID_CONFIG yet? But that seems unlikely, you have to be prepared to see attributes bigger than what you expected and skip them?
Yeah, I've already updated our internal code to filter netlink type using NLA_TYPE_MASK. Works like a charm now. Our codebase was initially made during kernel 3.x time when that filtering was not needed.. and within OpenWRT, the wireless stack got updated to 6.1.x bringing in mbssid netlink from 5.16. So this popped up suddenly :-)
Thanks for the patience and sorry for the noise!
johannes
linux-stable-mirror@lists.linaro.org
-
Johannes Berg -
Koen Vandeputte