cfg80211: make spurious warnings less likely, configurable

Bob reported that he got warnings in IBSS mode about
the ssid_len being zero on a joined event, but only
when kmemcheck was enabled. This appears to be due
to a race condition between drivers and userspace,
when the driver reports joined but the user in the
meantime decided to leave the IBSS again, the warning
would trigger. This was made more likely by kmemcheck
delaying the code that does the check and sends the
event.

So first, make the warning trigger closer to the
driver, which means it's not locked, but since only
the warning depends on it that's ok.

And secondly, users will not want to have spurious
warnings at all, so make those that are known to be
racy in such a way configurable.

Reported-by: Bob Copeland <me@bobcopeland.com>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index c6031d5b135fe6833df788c8f9ffc4e38b861e56..aea7e6824af9d2387d728770d1f9bdaad7c19a67 100644 (file)
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -17,6 +17,23 @@ config NL80211_TESTMODE
 
          Say N.
 
+config CFG80211_DEVELOPER_WARNINGS
+       bool "enable developer warnings"
+       depends on CFG80211
+       default n
+       help
+         This option enables some additional warnings that help
+         cfg80211 developers and driver developers, but that can
+         trigger due to races with userspace.
+
+         For example, when a driver reports that it was disconnected
+         from the AP, but the user disconnects manually at the same
+         time, the warning might trigger spuriously due to races.
+
+         Say Y only if you are developing cfg80211 or a driver based
+         on it (or mac80211).
+
+
 config CFG80211_REG_DEBUG
        bool "cfg80211 regulatory debugging"
        depends on CFG80211

diff --git a/net/wireless/core.h b/net/wireless/core.h
index 68eaf340d6136e310c50ec8c4a95106e95a72fe0..d262d42cbd5eb371a771f13132292ac2166b7063 100644 (file)
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -380,4 +380,15 @@ int rdev_set_freq(struct cfg80211_registered_device *rdev,
                  struct wireless_dev *for_wdev,
                  int freq, enum nl80211_channel_type channel_type);
 
+#ifdef CONFIG_CFG80211_DEVELOPER_WARNINGS
+#define CFG80211_DEV_WARN_ON(cond)     WARN_ON(cond)
+#else
+/*
+ * Trick to enable using it as a condition,
+ * and also not give a warning when it's
+ * not used that way.
+ */
+#define CFG80211_DEV_WARN_ON(cond)     ({bool __r = (cond); __r; })
+#endif
+
 #endif /* __NET_WIRELESS_CORE_H */

diff --git a/net/wireless/ibss.c b/net/wireless/ibss.c
index 42840a01be7405185b5e00785837d5ed09e9cac8..c88338911979363d646ef54c8da9394478e45290 100644 (file)
--- a/net/wireless/ibss.c
+++ b/net/wireless/ibss.c
@@ -22,7 +22,7 @@ void __cfg80211_ibss_joined(struct net_device *dev, const u8 *bssid)
        if (WARN_ON(wdev->iftype != NL80211_IFTYPE_ADHOC))
                return;
 
-       if (WARN_ON(!wdev->ssid_len))
+       if (!wdev->ssid_len)
                return;
 
        bss = cfg80211_get_bss(wdev->wiphy, NULL, bssid,
@@ -58,6 +58,8 @@ void cfg80211_ibss_joined(struct net_device *dev, const u8 *bssid, gfp_t gfp)
        struct cfg80211_event *ev;
        unsigned long flags;
 
+       CFG80211_DEV_WARN_ON(!wdev->ssid_len);
+
        ev = kzalloc(sizeof(*ev), gfp);
        if (!ev)
                return;

diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 4a8289f9b4f01c3602c205047e1c9deabca85534..68307883ec87ad36145ce6f00e63eb2c9fcb64ed 100644 (file)
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -351,7 +351,7 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
        if (WARN_ON(wdev->iftype != NL80211_IFTYPE_STATION))
                return;
 
-       if (WARN_ON(wdev->sme_state != CFG80211_SME_CONNECTING))
+       if (wdev->sme_state != CFG80211_SME_CONNECTING)
                return;
 
        nl80211_send_connect_result(wiphy_to_dev(wdev->wiphy), dev,
@@ -445,6 +445,8 @@ void cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
        struct cfg80211_event *ev;
        unsigned long flags;
 
+       CFG80211_DEV_WARN_ON(wdev->sme_state != CFG80211_SME_CONNECTING);
+
        ev = kzalloc(sizeof(*ev) + req_ie_len + resp_ie_len, gfp);
        if (!ev)
                return;
@@ -481,7 +483,7 @@ void __cfg80211_roamed(struct wireless_dev *wdev, const u8 *bssid,
        if (WARN_ON(wdev->iftype != NL80211_IFTYPE_STATION))
                return;
 
-       if (WARN_ON(wdev->sme_state != CFG80211_SME_CONNECTED))
+       if (wdev->sme_state != CFG80211_SME_CONNECTED)
                return;
 
        /* internal error -- how did we get to CONNECTED w/o BSS? */
@@ -540,6 +542,8 @@ void cfg80211_roamed(struct net_device *dev, const u8 *bssid,
        struct cfg80211_event *ev;
        unsigned long flags;
 
+       CFG80211_DEV_WARN_ON(wdev->sme_state != CFG80211_SME_CONNECTED);
+
        ev = kzalloc(sizeof(*ev) + req_ie_len + resp_ie_len, gfp);
        if (!ev)
                return;
@@ -575,7 +579,7 @@ void __cfg80211_disconnected(struct net_device *dev, const u8 *ie,
        if (WARN_ON(wdev->iftype != NL80211_IFTYPE_STATION))
                return;
 
-       if (WARN_ON(wdev->sme_state != CFG80211_SME_CONNECTED))
+       if (wdev->sme_state != CFG80211_SME_CONNECTED)
                return;
 
        if (wdev->current_bss) {
@@ -639,6 +643,8 @@ void cfg80211_disconnected(struct net_device *dev, u16 reason,
        struct cfg80211_event *ev;
        unsigned long flags;
 
+       CFG80211_DEV_WARN_ON(wdev->sme_state != CFG80211_SME_CONNECTED);
+
        ev = kzalloc(sizeof(*ev) + ie_len, gfp);
        if (!ev)
                return;