Skip to contentSkip to conversation

Regression caused by commit df88129

PATCH In review Larry Finger 3 participants 8 messages Feb 17, 2013 – Feb 18, 2013 802.11 (including cfg80211/NL80211)

lore
Share & tools
lore
#0 lore raw

Jouni and Johannes,

A recent pull of the wireless-testing tree caused my wireless to break. The wireless devices can authenticate and associate as follows:

wlan0: authenticate with c0:3f:0e:be:2b:44
wlan0: capabilities/regulatory prevented using AP HT/VHT configuration, downgraded
wlan0: send auth to c0:3f:0e:be:2b:44 (try 1/3)
wlan0: authenticated
wlan0: associate with c0:3f:0e:be:2b:44 (try 1/3)
wlan0: RX AssocResp from c0:3f:0e:be:2b:44 (capab=0x411 status=0 aid=3)
wlan0: associated
IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
b43-phy0 debug: Using hardware based encryption for keyidx: 0, mac: c0:3f:0e:be:2b:44 b43-phy0 debug: Using hardware based encryption for keyidx: 1, mac: ff:ff:ff:ff:ff:ff
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

These dropped frame messages persist, and prevent the connection from being completed. The code in question is in routine ieee80211_subif_start_xmit() of file net/mac80211/tx.c as follows:

==============================================================

        if (unlikely(!ieee80211_vif_is_mesh(&sdata->vif) &&
                     !is_multicast_ether_addr(hdr.addr1) && !authorized &&
                     (cpu_to_be16(ethertype) != sdata->control_port_protocol ||
                      !ether_addr_equal(sdata->vif.addr, skb->data + ETH_ALEN)))) {
#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
                net_info_ratelimited("%s: dropped frame to %pM (unauthorized 
port)\n",
                                    dev->name, hdr.addr1);

#endif

                I802_DEBUG_INC(local->tx_handlers_drop_unauth_port);

                goto fail_rcu;
        }

==============================================================

When the if statement is rewritten as 'if (A || B)', it turns out that A is true, and B is false.

The regression was bisected to commit df881293c6ba9a12868491a717b25cb14ec1fa4a.

==============================================================
commit df881293c6ba9a12868491a717b25cb14ec1fa4a

Author: Jouni Malinen <[email protected]>
Date:   Thu Feb 14 21:10:54 2013 +0200

cfg80211: Pass TDLS peer's QoS/HT/VHT information during set_station

The information of the peer's capabilities is required for the driver to perform TDLS Peer UAPSD operations. This information of the peer is passed by the supplicant using NL80211_CMD_SET_STATION command. This commit enhances the function nl80211_set_station to pass this information of the peer to the driver in case this command is used with the TDLS peer STA.

In addition, make the HT/VHT capability configuration handled more consistently for other STA cases (reject both instead of just HT).

==============================================================

If you need any more debugging output for this problem, please let me know.

Thanks,

Larry

#1 lore raw
Quoted 8 lines · Larry Finger

A recent pull of the wireless-testing tree caused my wireless to break. The
wireless devices can authenticate and associate as follows:

wlan0: RX AssocResp from c0:3f:0e:be:2b:44 (capab=0x411 status=0 aid=3)
wlan0: associated
IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

Which wpa_supplicant version are you using here? I'm assuming this is with WPA2.

It sounds like the AP STA entry does not get authorized properly. I did see some issues when working on the patch, but fixed those. The version that was
sent out should have worked fine since I did manage to complete TDLS setup with it (i.e., send and receive frames through the AP to another STA).

I will be flying for the rest of Sunday, so may not be able to do much about this before Monday.

- Jouni

#2 lore raw
In reply to Larry Finger
Quoted 2 lines · Larry Finger

IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

I do see one of these (which, I'd assume, is actually expected if you have something on the system that runs to transmit before 4-way handshake is completed).

Quoted 2 lines

wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

But this does not show up in my tests with mac80211_hwsim and the current wireless-testing.git and hostap.git snapshots.

Quoted 2 lines

The regression was bisected to commit
df881293c6ba9a12868491a717b25cb14ec1fa4a.

Did you confirm that the latest snapshot works with this reverted?

Quoted 2 lines

If you need any more debugging output for this problem, please let me know.

Since I was unable to reproduce, some more debug output could be useful. Please at least verify that the STA entry for the AP is not marked authorized. The next step would be to check whether sta_set_flags in wpa_supplicant driver_nl80211.c is failing. Unfortunately, it looks like the current implementation does not make this very clear in the debug log, so some additional prints may be needed there.

Please also describe the security configuration (WPA2-Personal/CCMP?) that you are using.

- Jouni

#3 lore raw
Quoted 10 lines · Jouni
Quoted 2 lines · Larry Finger

IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

I do see one of these (which, I'd assume, is actually expected if you have something on the system that runs to transmit before 4-way handshake is completed).

I also have seen one of these for some time.

Quoted 10 lines
Quoted 2 lines

wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

But this does not show up in my tests with mac80211_hwsim and the current wireless-testing.git and hostap.git snapshots.

Quoted 2 lines

The regression was bisected to commit
df881293c6ba9a12868491a717b25cb14ec1fa4a.

Did you confirm that the latest snapshot works with this reverted?

With the commit reverted, the latest kernel-wireless version works.

Quoted 9 lines
Quoted 2 lines

If you need any more debugging output for this problem, please let me know.

Since I was unable to reproduce, some more debug output could be useful. Please at least verify that the STA entry for the AP is not marked authorized. The next step would be to check whether sta_set_flags in wpa_supplicant driver_nl80211.c is failing. Unfortunately, it looks like the current implementation does not make this very clear in the debug log, so some additional prints may be needed there.

Just to be clear, b43 is used as a station, not as AP. The card I am using has a G PHY. I added printk statements for every setting of 'authorized' in net/mac80211/tx.c. It is set false at line 1984, thus 'multicast' is false. Furthermore, sta is not NULL, but WLAN_STA_AUTHORIZED is not set in the sta flags. Note: sta->_flags is 0x60023.

Although sdata->vif.type is set to 2 (NL80211_IFTYPE_STATION), variable 'authorized' was not touched at line 1908, thus "sdata->wdev.wiphy->flags & WIPHY_FLAG_SUPPORTS_TDLS" is false. The content of sdata->wdev.wiphy->flags is 0x1401f8.

Quoted 2 lines

Please also describe the security configuration (WPA2-Personal/CCMP?) that you are using.

I'm running openSUSE 12.3 RC1, which has version 1.1 of wpa_supplicant. The router is a Netgear WNDR3300 running standard firmware and set for WPA2-PSK(AES) encryption.

Checking with newer versions of the supplicant is being a problem. I built both 2.0 from the tarball and 2.1-devel from the git repo, but nether will run with NetworkManager. I will see if I can resolve that problem later.

Thanks,

Larry

maintainer
#4 lore raw
In reply to Larry Finger
Quoted 5 lines · Larry Finger

Jouni and Johannes,

A recent pull of the wireless-testing tree caused my wireless to break. The wireless devices can authenticate and associate as follows:

Quoted 15 lines

wlan0: authenticate with c0:3f:0e:be:2b:44
wlan0: capabilities/regulatory prevented using AP HT/VHT configuration, downgraded
wlan0: send auth to c0:3f:0e:be:2b:44 (try 1/3)
wlan0: authenticated
wlan0: associate with c0:3f:0e:be:2b:44 (try 1/3)
wlan0: RX AssocResp from c0:3f:0e:be:2b:44 (capab=0x411 status=0 aid=3)
wlan0: associated
IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port) b43-phy0 debug: Using hardware based encryption for keyidx: 0, mac:
c0:3f:0e:be:2b:44
b43-phy0 debug: Using hardware based encryption for keyidx: 1, mac:
ff:ff:ff:ff:ff:ff
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

Yeah, it's obvious that this would happen for drivers not supporting TDLS -- will send a patch.

johannes

maintainer
#5 lore raw

PATCH 1/1 cfg80211: fix station change if TDLS isn't supported

In reply to Larry Finger

From: Johannes Berg <[email protected]>

Larry noticed (and bisected) that commit df881293c6ba9a12868491a717b25 "cfg80211: Pass TDLS peer's QoS/HT/VHT information during set_station" broke secure connections. This is is the case only for drivers that don't support TDLS, where any kind of change, even just the change of authorized flag that is required for normal operation, was rejected now. To fix this, remove the checks. I have some patches that will add proper verification for all the different cases later.

---
 net/wireless/nl80211.c | 8 --------
 1 file changed, 8 deletions(-)
net/wireless/nl80211.c +0 -8
index 580ffea..d86af75 100644
@@ -3423,14 +3423,6 @@ static int nl80211_set_station_tdls(struct genl_info *info,
 	struct nlattr *nla;
 	int err;
 
-	/* Can only set if TDLS ... */
-	if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_TDLS))
-		return -EOPNOTSUPP;
-
-	/* ... with external setup is supported */
-	if (!(rdev->wiphy.flags & WIPHY_FLAG_TDLS_EXTERNAL_SETUP))
-		return -EOPNOTSUPP;
-
 	/* Dummy STA entry gets updated once the peer capabilities are known */
 	if (info->attrs[NL80211_ATTR_HT_CAPABILITY])
 		params->ht_capa =
Signature · 1 line
1.8.0
author Tested-by on v1 1/1
#6 lore raw

Re: [PATCH] cfg80211: fix station change if TDLS isn't supported

Quoted 10 lines · Johannes Berg

From: Johannes Berg <[email protected]>

Larry noticed (and bisected) that commit df881293c6ba9a12868491a717b25 "cfg80211: Pass TDLS peer's QoS/HT/VHT information during set_station" broke secure connections. This is is the case only for drivers that don't support TDLS, where any kind of change, even just the change of authorized flag that is required for normal operation, was rejected now. To fix this, remove the checks. I have some patches that will add proper verification for all the different cases later.

Quoted 3 lines

Johannes,

Thanks. This patch does fix the problem. The only thing I saw was an unused variable:

net/wireless/nl80211.c: In function ‘nl80211_set_station_tdls’:
net/wireless/nl80211.c:3421:37: warning: unused variable ‘rdev’ [-Wunused-variable]

Larry

Quoted 24 lines
---
  net/wireless/nl80211.c | 8 --------
  1 file changed, 8 deletions(-)
net/wireless/nl80211.c +0 -8
index 580ffea..d86af75 100644
@@ -3423,14 +3423,6 @@ static int nl80211_set_station_tdls(struct genl_info *info,
  	struct nlattr *nla;
  	int err;
 
-	/* Can only set if TDLS ... */
-	if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_TDLS))
-		return -EOPNOTSUPP;
-
-	/* ... with external setup is supported */
-	if (!(rdev->wiphy.flags & WIPHY_FLAG_TDLS_EXTERNAL_SETUP))
-		return -EOPNOTSUPP;
-
  	/* Dummy STA entry gets updated once the peer capabilities are known */
  	if (info->attrs[NL80211_ATTR_HT_CAPABILITY])
  		params->ht_capa =
maintainer
#7 lore raw

Re: [PATCH] cfg80211: fix station change if TDLS isn't supported

Quoted 17 lines · Larry Finger
Quoted 10 lines · Johannes Berg

From: Johannes Berg <[email protected]>

Larry noticed (and bisected) that commit df881293c6ba9a12868491a717b25 "cfg80211: Pass TDLS peer's QoS/HT/VHT information during set_station" broke secure connections. This is is the case only for drivers that don't support TDLS, where any kind of change, even just the change of authorized flag that is required for normal operation, was rejected now. To fix this, remove the checks. I have some patches that will add proper verification for all the different cases later.

Quoted 3 lines

Johannes,

Quoted 7 lines

Thanks. This patch does fix the problem. The only thing I saw was an unused variable:

net/wireless/nl80211.c: In function ‘nl80211_set_station_tdls’:
net/wireless/nl80211.c:3421:37: warning: unused variable ‘rdev’ [-Wunused-variable]

Thanks, I'll fix that variable and apply the patch.

johannes