diff mbox series

cfg80211: Fix spamming of logs when number of scan results is limited

Message ID 20220217011226.31579-1-Larry.Finger@lwfinger.net (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show
Series cfg80211: Fix spamming of logs when number of scan results is limited | expand

Commit Message

Larry Finger Feb. 17, 2022, 1:12 a.m. UTC
When the cfg80211 option "bss_entries_limit" is set to 1, routine
cfg80211_bss_expire_oldest() fails by issuing repeated warnings.
The problem could also occur in the unlikely event that a scan only finds
a single SSID. In the first case, the warning is avoided by testing for
the special ivalue for the option before scanning for the oldest entry.
The second case is handled by converting the WARN_ON() to a WARN_ON_ONCE().
These changes fix commit 9853a55ef1bb ("cfg80211: limit scan results cache
size").

Reported-by: Maxim Klimenko Sergievich <klimenkomaximsergievich@gmail.com>
Fixes: 9853a55ef1bb ("cfg80211: limit scan results cache size").
Cc: stable@vger.kernel.org # 4.9+
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 net/wireless/scan.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Johannes Berg Aug. 26, 2022, 8:22 a.m. UTC | #1
Hi,

Sorry I didn't get to this for so long ... Didn't really give it
priority since it's such a strange special case configuration.

On Wed, 2022-02-16 at 19:12 -0600, Larry Finger wrote:
> When the cfg80211 option "bss_entries_limit" is set to 1, routine
> cfg80211_bss_expire_oldest() fails by issuing repeated warnings.

Yeah ... special configuration, not very sensible.

> The problem could also occur in the unlikely event that a scan only finds
> a single SSID.
> 

That's not true though, it could only happen if there are as many as
bss_entries_limit entries (which defaults to 1000!) that *all* have a
reason to be held in place (e.g. bss->hold, or part of a hidden SSID
BSS).

So to hit this with default settings, you'd have to have 1 network
connected, and 999 real SSIDs for it with other hidden SSIDs or
something like that?

Anyway, I think the right thing to do is to just remove the warning
completely, even if bss_entries_limit==1 is probably a config you don't
want to make.

johannes
diff mbox series

Patch

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 22e92be61938..d7aa38445fe6 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -463,6 +463,12 @@  static bool cfg80211_bss_expire_oldest(struct cfg80211_registered_device *rdev)
 
 	lockdep_assert_held(&rdev->bss_lock);
 
+	/* If the user has set cfg80211 option bss_entries_limit to 1,
+	 * there cannot be an oldest BSS. Skip the scan.
+	 */
+	if (unlikely(rdev->bss_entries == 1))
+		return false;
+
 	list_for_each_entry(bss, &rdev->bss_list, list) {
 		if (atomic_read(&bss->hold))
 			continue;
@@ -476,7 +482,7 @@  static bool cfg80211_bss_expire_oldest(struct cfg80211_registered_device *rdev)
 		oldest = bss;
 	}
 
-	if (WARN_ON(!oldest))
+	if (WARN_ON_ONCE(!oldest))
 		return false;
 
 	/*