The BSS lookup for an authentication or association request can fail for three distinct reasons: cfg80211 has no scan entry at all for the BSSID/channel, an entry exists but is older than IEEE80211_SCAN_RESULT_EXPIRE (and not held), or a fresh entry exists but its use_for flags do not allow this use. All three currently surface as the same generic extack message "Error fetching BSS for link" on the MLO association path, and as a bare -ENOENT with no message at all on the authentication and non-MLO association paths. Since wpa_supplicant logs the extack message verbatim ("nl80211: kernel reports: ..."), that message is often the only diagnostic a user sees when an MLO association degrades to fewer links, and it does not say whether a fresh scan could have helped. In practice the expired case is common for MLO partner links: 6 GHz is passive-scan in many regulatory domains, so the partner-link entry is routinely stale by the time userspace requests the association even though the link is perfectly usable. Let __cfg80211_get_bss() take an optional extack and record, during the same bss_lock walk that fails the lookup, whether any matching entry was rejected for being expired or for not being usable for the requested use, and set a distinct message for each case (and a combined one when different entries were rejected for different reasons). Reorder the checks in the walk so that an entry's identity (type, privacy, channel, BSSID/SSID) is established before the usability checks; this doesn't change which entry is returned since an entry is only used when all checks pass. Also give the -EINVAL paths in nl80211_assoc_bss() proper messages while at it, and keep pointing the bad_attr at the failing link on the MLO path there; the message for that case is already set by the lookup itself. Signed-off-by: Louis Kotze --- v3: use GENL_SET_ERR_MSG for the -EINVAL messages; comment the bare NL_SET_BAD_ATTR (the lookup already set the specific message); comment the check ordering in the walk; combined message when matching entries were rejected for different reasons; drop the "scan again" instruction; minimal diff at the nl80211_associate() call site. v2: capture the reason inside __cfg80211_get_bss() during the single bss_lock walk (per Johannes' feedback on v1); cover the auth path; KUnit test added as patch 2. include/net/cfg80211.h | 7 +++++-- net/wireless/nl80211.c | 27 +++++++++++++++--------- net/wireless/scan.c | 44 ++++++++++++++++++++++++++++++++------- net/wireless/tests/scan.c | 2 +- 4 files changed, 59 insertions(+), 21 deletions(-) diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index b8e9fbb89e69..15c08b24502f 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -8424,6 +8424,8 @@ cfg80211_inform_bss(struct wiphy *wiphy, * @bss_type: type of BSS, see &enum ieee80211_bss_type * @privacy: privacy filter, see &enum ieee80211_privacy * @use_for: indicates which use is intended + * @extack: (optional) extack that is filled with the reason when no + * usable entry was found; may be %NULL * * Return: Reference-counted BSS on success. %NULL on error. */ @@ -8433,7 +8435,8 @@ struct cfg80211_bss *__cfg80211_get_bss(struct wiphy *wiphy, const u8 *ssid, size_t ssid_len, enum ieee80211_bss_type bss_type, enum ieee80211_privacy privacy, - u32 use_for); + u32 use_for, + struct netlink_ext_ack *extack); /** * cfg80211_get_bss - get a BSS reference @@ -8457,7 +8460,7 @@ cfg80211_get_bss(struct wiphy *wiphy, struct ieee80211_channel *channel, { return __cfg80211_get_bss(wiphy, channel, bssid, ssid, ssid_len, bss_type, privacy, - NL80211_BSS_USE_FOR_NORMAL); + NL80211_BSS_USE_FOR_NORMAL, NULL); } static inline struct cfg80211_bss * diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index d962b5944533..ac0c0da45241 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -12890,9 +12890,11 @@ static int nl80211_authenticate(struct sk_buff *skb, struct genl_info *info) return -EINVAL; } - req.bss = cfg80211_get_bss(&rdev->wiphy, chan, bssid, ssid, ssid_len, - IEEE80211_BSS_TYPE_ESS, - IEEE80211_PRIVACY_ANY); + req.bss = __cfg80211_get_bss(&rdev->wiphy, chan, bssid, ssid, ssid_len, + IEEE80211_BSS_TYPE_ESS, + IEEE80211_PRIVACY_ANY, + NL80211_BSS_USE_FOR_NORMAL, + info->extack); if (!req.bss) return -ENOENT; @@ -13037,6 +13039,7 @@ static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev, } static struct cfg80211_bss *nl80211_assoc_bss(struct cfg80211_registered_device *rdev, + struct genl_info *info, const u8 *ssid, int ssid_len, struct nlattr **attrs, int assoc_link_id, int link_id) @@ -13046,8 +13049,10 @@ static struct cfg80211_bss *nl80211_assoc_bss(struct cfg80211_registered_device const u8 *bssid; u32 freq, use_for = 0; - if (!attrs[NL80211_ATTR_MAC] || !attrs[NL80211_ATTR_WIPHY_FREQ]) + if (!attrs[NL80211_ATTR_MAC] || !attrs[NL80211_ATTR_WIPHY_FREQ]) { + GENL_SET_ERR_MSG(info, "BSSID or frequency missing"); return ERR_PTR(-EINVAL); + } bssid = nla_data(attrs[NL80211_ATTR_MAC]); @@ -13056,8 +13061,10 @@ static struct cfg80211_bss *nl80211_assoc_bss(struct cfg80211_registered_device freq += nla_get_u32(attrs[NL80211_ATTR_WIPHY_FREQ_OFFSET]); chan = nl80211_get_valid_chan(&rdev->wiphy, freq); - if (!chan) + if (!chan) { + GENL_SET_ERR_MSG(info, "invalid or disabled channel"); return ERR_PTR(-EINVAL); + } if (assoc_link_id >= 0) use_for = NL80211_BSS_USE_FOR_MLD_LINK; @@ -13068,7 +13075,7 @@ static struct cfg80211_bss *nl80211_assoc_bss(struct cfg80211_registered_device ssid, ssid_len, IEEE80211_BSS_TYPE_ESS, IEEE80211_PRIVACY_ANY, - use_for); + use_for, info->extack); if (!bss) return ERR_PTR(-ENOENT); @@ -13107,13 +13114,13 @@ static int nl80211_process_links(struct cfg80211_registered_device *rdev, return -EINVAL; } links[link_id].bss = - nl80211_assoc_bss(rdev, ssid, ssid_len, attrs, + nl80211_assoc_bss(rdev, info, ssid, ssid_len, attrs, assoc_link_id, link_id); if (IS_ERR(links[link_id].bss)) { err = PTR_ERR(links[link_id].bss); links[link_id].bss = NULL; - NL_SET_ERR_MSG_ATTR(info->extack, link, - "Error fetching BSS for link"); + /* the BSS lookup set the specific message already */ + NL_SET_BAD_ATTR(info->extack, link); return err; } @@ -13329,7 +13336,7 @@ static int nl80211_associate(struct sk_buff *skb, struct genl_info *info) if (req.link_id >= 0) return -EINVAL; - req.bss = nl80211_assoc_bss(rdev, ssid, ssid_len, info->attrs, + req.bss = nl80211_assoc_bss(rdev, info, ssid, ssid_len, info->attrs, -1, -1); if (IS_ERR(req.bss)) return PTR_ERR(req.bss); diff --git a/net/wireless/scan.c b/net/wireless/scan.c index e62b7dd2b7c2..90a4f285654f 100644 --- a/net/wireless/scan.c +++ b/net/wireless/scan.c @@ -1609,10 +1609,12 @@ struct cfg80211_bss *__cfg80211_get_bss(struct wiphy *wiphy, const u8 *ssid, size_t ssid_len, enum ieee80211_bss_type bss_type, enum ieee80211_privacy privacy, - u32 use_for) + u32 use_for, + struct netlink_ext_ack *extack) { struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy); struct cfg80211_internal_bss *bss, *res = NULL; + bool expired = false, unusable = false; unsigned long now = jiffies; int bss_privacy; @@ -1634,22 +1636,48 @@ struct cfg80211_bss *__cfg80211_get_bss(struct wiphy *wiphy, continue; if (!is_valid_ether_addr(bss->pub.bssid)) continue; - if ((bss->pub.use_for & use_for) != use_for) + if (!is_bss(&bss->pub, bssid, ssid, ssid_len)) continue; + + /* + * The identity checks above must all come first so that + * the expired/unusable classification below only ever + * applies to entries that actually match the request. + */ + /* Don't get expired BSS structs */ if (time_after(now, bss->ts + IEEE80211_SCAN_RESULT_EXPIRE) && - !atomic_read(&bss->hold)) + !atomic_read(&bss->hold)) { + expired = true; + continue; + } + + if ((bss->pub.use_for & use_for) != use_for) { + unusable = true; continue; - if (is_bss(&bss->pub, bssid, ssid, ssid_len)) { - res = bss; - bss_ref_get(rdev, res); - break; } + + res = bss; + bss_ref_get(rdev, res); + break; } spin_unlock_bh(&rdev->bss_lock); - if (!res) + if (!res) { + if (expired && unusable) + NL_SET_ERR_MSG(extack, + "BSS entries are expired or cannot be used for the requested operation"); + else if (unusable) + NL_SET_ERR_MSG(extack, + "BSS cannot be used for the requested operation"); + else if (expired) + NL_SET_ERR_MSG(extack, + "BSS entry in scan results is expired"); + else + NL_SET_ERR_MSG(extack, + "BSS not found in scan results"); return NULL; + } trace_cfg80211_return_bss(&res->pub); return &res->pub; } diff --git a/net/wireless/tests/scan.c b/net/wireless/tests/scan.c index b1a9c1466d6c..2fc717317ac3 100644 --- a/net/wireless/tests/scan.c +++ b/net/wireless/tests/scan.c @@ -617,7 +617,7 @@ static void test_inform_bss_ml_sta(struct kunit *test) link_bss = __cfg80211_get_bss(wiphy, NULL, sta_prof.bssid, NULL, 0, IEEE80211_BSS_TYPE_ANY, IEEE80211_PRIVACY_ANY, - 0); + 0, NULL); KUNIT_ASSERT_NOT_NULL(test, link_bss); KUNIT_EXPECT_EQ(test, link_bss->signal, 0); KUNIT_EXPECT_EQ(test, link_bss->beacon_interval, -- 2.55.0 Add a KUnit test for the extack failure reasons that __cfg80211_get_bss() now reports: no matching scan entry at all, a matching entry that is expired, and a matching entry whose use_for flags do not allow the requested use. Also cover the cases that must not report a failure (a fresh entry, and an expired-but-held entry), an entry that is both expired and unusable, and the combined message when one matching entry is expired while another is current but unusable. Signed-off-by: Louis Kotze --- v3: message strings updated to match patch 1; the cross-entry case now checks the combined message. net/wireless/tests/scan.c | 119 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/net/wireless/tests/scan.c b/net/wireless/tests/scan.c index 2fc717317ac3..8c20278b5d3a 100644 --- a/net/wireless/tests/scan.c +++ b/net/wireless/tests/scan.c @@ -402,6 +402,124 @@ static void test_inform_bss_ssid_only(struct kunit *test) cfg80211_put_bss(wiphy, bss); } +static void test_get_bss_miss_reason(struct kunit *test) +{ + struct inform_bss ctx = { + .test = test, + }; + struct wiphy *wiphy = T_WIPHY(test, ctx); + struct cfg80211_inform_bss inform_bss = { + .signal = 50, + .drv_data = &ctx, + }; + const u8 bssid[ETH_ALEN] = { 0x10, 0x22, 0x33, 0x44, 0x55, 0x66 }; + const u8 other_bssid[ETH_ALEN] = { 0x66, 0x55, 0x44, 0x33, 0x22, 0x11 }; + static const u8 ies[] = { + [0] = WLAN_EID_SSID, + [1] = 4, + [2] = 'T', 'E', 'S', 'T' + }; + struct cfg80211_internal_bss *ibss; + struct netlink_ext_ack extack = {}; + struct cfg80211_bss *bss, *bss2, *found; + + inform_bss.chan = ieee80211_get_channel_khz(wiphy, MHZ_TO_KHZ(2412)); + KUNIT_ASSERT_NOT_NULL(test, inform_bss.chan); + + bss = cfg80211_inform_bss_data(wiphy, &inform_bss, + CFG80211_BSS_FTYPE_PRESP, bssid, 0, + 0x1234, 100, ies, sizeof(ies), + GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, bss); + ibss = container_of(bss, struct cfg80211_internal_bss, pub); + + /* Fresh usable entry: found, no message is set */ + found = __cfg80211_get_bss(wiphy, NULL, bssid, NULL, 0, + IEEE80211_BSS_TYPE_ANY, + IEEE80211_PRIVACY_ANY, + NL80211_BSS_USE_FOR_NORMAL, &extack); + KUNIT_ASSERT_PTR_EQ(test, found, bss); + KUNIT_EXPECT_NULL(test, extack._msg); + cfg80211_put_bss(wiphy, found); + + /* No entry at all for this BSSID */ + found = __cfg80211_get_bss(wiphy, NULL, other_bssid, NULL, 0, + IEEE80211_BSS_TYPE_ANY, + IEEE80211_PRIVACY_ANY, + NL80211_BSS_USE_FOR_NORMAL, &extack); + KUNIT_EXPECT_NULL(test, found); + KUNIT_EXPECT_STREQ(test, extack._msg, "BSS not found in scan results"); + + /* Fresh entry that is not usable for the requested use */ + extack._msg = NULL; + bss->use_for = 0; + found = __cfg80211_get_bss(wiphy, NULL, bssid, NULL, 0, + IEEE80211_BSS_TYPE_ANY, + IEEE80211_PRIVACY_ANY, + NL80211_BSS_USE_FOR_NORMAL, &extack); + KUNIT_EXPECT_NULL(test, found); + KUNIT_EXPECT_STREQ(test, extack._msg, + "BSS cannot be used for the requested operation"); + bss->use_for = NL80211_BSS_USE_FOR_ALL; + + /* Expired entry, > IEEE80211_SCAN_RESULT_EXPIRE (30s) old */ + extack._msg = NULL; + ibss->ts = jiffies - 60 * HZ; + found = __cfg80211_get_bss(wiphy, NULL, bssid, NULL, 0, + IEEE80211_BSS_TYPE_ANY, + IEEE80211_PRIVACY_ANY, + NL80211_BSS_USE_FOR_NORMAL, &extack); + KUNIT_EXPECT_NULL(test, found); + KUNIT_EXPECT_STREQ(test, extack._msg, + "BSS entry in scan results is expired"); + + /* An entry both expired and unusable reports expired */ + extack._msg = NULL; + bss->use_for = 0; + found = __cfg80211_get_bss(wiphy, NULL, bssid, NULL, 0, + IEEE80211_BSS_TYPE_ANY, + IEEE80211_PRIVACY_ANY, + NL80211_BSS_USE_FOR_NORMAL, &extack); + KUNIT_EXPECT_NULL(test, found); + KUNIT_EXPECT_STREQ(test, extack._msg, + "BSS entry in scan results is expired"); + bss->use_for = NL80211_BSS_USE_FOR_ALL; + + /* Expired but held entries are still usable, no message is set */ + extack._msg = NULL; + atomic_set(&ibss->hold, 1); + found = __cfg80211_get_bss(wiphy, NULL, bssid, NULL, 0, + IEEE80211_BSS_TYPE_ANY, + IEEE80211_PRIVACY_ANY, + NL80211_BSS_USE_FOR_NORMAL, &extack); + KUNIT_ASSERT_PTR_EQ(test, found, bss); + KUNIT_EXPECT_NULL(test, extack._msg); + cfg80211_put_bss(wiphy, found); + atomic_set(&ibss->hold, 0); + + /* + * With one matching entry expired and another current but + * unusable, both reasons are reported. + */ + bss2 = cfg80211_inform_bss_data(wiphy, &inform_bss, + CFG80211_BSS_FTYPE_PRESP, other_bssid, + 0, 0x1234, 100, ies, sizeof(ies), + GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, bss2); + bss2->use_for = 0; + extack._msg = NULL; + found = __cfg80211_get_bss(wiphy, NULL, NULL, "TEST", 4, + IEEE80211_BSS_TYPE_ANY, + IEEE80211_PRIVACY_ANY, + NL80211_BSS_USE_FOR_NORMAL, &extack); + KUNIT_EXPECT_NULL(test, found); + KUNIT_EXPECT_STREQ(test, extack._msg, + "BSS entries are expired or cannot be used for the requested operation"); + + cfg80211_put_bss(wiphy, bss2); + cfg80211_put_bss(wiphy, bss); +} + static struct inform_bss_ml_sta_case { const char *desc; int mld_id; @@ -855,6 +973,7 @@ kunit_test_suite(gen_new_ie); static struct kunit_case inform_bss_test_cases[] = { KUNIT_CASE(test_inform_bss_ssid_only), + KUNIT_CASE(test_get_bss_miss_reason), KUNIT_CASE_PARAM(test_inform_bss_ml_sta, inform_bss_ml_sta_gen_params), {} }; -- 2.55.0