mac80211: always wind down STA state

Commit Message

Johannes Berg Oct. 9, 2020, 12:17 p.m. UTC

From: Johannes Berg <[e-mail]>

When (for example) an IBSS station is pre-moved to AUTHORIZED
before it's inserted, and then the insertion fails, we don't
clean up the fast RX/TX states that might already have been
created, since we don't go through all the state transitions
again on the way down.

Do that, if it hasn't been done already, when the station is
freed. I considered only freeing the fast TX/RX state there,
but we might add more state so it's more robust to wind down
the state properly.

Note that we warn if the station was ever inserted, it should
have been properly cleaned up in that case, and the driver
will probably not like things happening out of order.

Reported-by: [e-mail]
Signed-off-by: Johannes Berg <[e-mail]>
---
 net/mac80211/sta_info.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

kernel test robot Oct. 9, 2020, 2:18 p.m. UTC | #1

Hi Johannes,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mac80211-next/master]
[also build test WARNING on mac80211/master v5.9-rc8 next-20201009]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Johannes-Berg/mac80211-always-wind-down-STA-state/20201009-201908
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/aaf96fee9f08499876d52ec4861f504bb1ff6143
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Johannes-Berg/mac80211-always-wind-down-STA-state/20201009-201908
        git checkout aaf96fee9f08499876d52ec4861f504bb1ff6143
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[e-mail]>

All warnings (new ones prefixed by >>):

   net/mac80211/sta_info.c: In function 'sta_info_free':
>> net/mac80211/sta_info.c:272:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     272 |   int ret = sta_info_move_state(sta, sta->sta_state - 1);
         |   ^~~

vim +272 net/mac80211/sta_info.c

   247	
   248	/**
   249	 * sta_info_free - free STA
   250	 *
   251	 * @local: pointer to the global information
   252	 * @sta: STA info to free
   253	 *
   254	 * This function must undo everything done by sta_info_alloc()
   255	 * that may happen before sta_info_insert(). It may only be
   256	 * called when sta_info_insert() has not been attempted (and
   257	 * if that fails, the station is freed anyway.)
   258	 */
   259	void sta_info_free(struct ieee80211_local *local, struct sta_info *sta)
   260	{
   261		/*
   262		 * If we had used sta_info_pre_move_state() then we might not
   263		 * have gone through the state transitions down again, so do
   264		 * it here now (and warn if it's inserted).
   265		 *
   266		 * This will clear state such as fast TX/RX that may have been
   267		 * allocated during state transitions.
   268		 */
   269		while (sta->sta_state > IEEE80211_STA_NONE) {
   270			WARN_ON_ONCE(test_sta_flag(sta, WLAN_STA_INSERTED));
   271	
 > 272			int ret = sta_info_move_state(sta, sta->sta_state - 1);
   273			if (ret) {
   274				WARN_ON_ONCE(1);
   275				break;
   276			}
   277		}
   278	
   279		if (sta->rate_ctrl)
   280			rate_control_free_sta(sta);
   281	
   282		sta_dbg(sta->sdata, "Destroyed STA %pM\n", sta->sta.addr);
   283	
   284		if (sta->sta.txq[0])
   285			kfree(to_txq_info(sta->sta.txq[0]));
   286		kfree(rcu_dereference_raw(sta->sta.rates));
   287	#ifdef CONFIG_MAC80211_MESH
   288		kfree(sta->mesh);
   289	#endif
   290		free_percpu(sta->pcpu_rx_stats);
   291		kfree(sta);
   292	}
   293	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[e-mail]

Patch

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index f2840d1d95cf..117311f6f6b3 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -258,6 +258,24 @@  struct sta_info *sta_info_get_by_idx(struct ieee80211_sub_if_data *sdata,
  */
 void sta_info_free(struct ieee80211_local *local, struct sta_info *sta)
 {
+	/*
+	 * If we had used sta_info_pre_move_state() then we might not
+	 * have gone through the state transitions down again, so do
+	 * it here now (and warn if it's inserted).
+	 *
+	 * This will clear state such as fast TX/RX that may have been
+	 * allocated during state transitions.
+	 */
+	while (sta->sta_state > IEEE80211_STA_NONE) {
+		WARN_ON_ONCE(test_sta_flag(sta, WLAN_STA_INSERTED));
+
+		int ret = sta_info_move_state(sta, sta->sta_state - 1);
+		if (ret) {
+			WARN_ON_ONCE(1);
+			break;
+		}
+	}
+
 	if (sta->rate_ctrl)
 		rate_control_free_sta(sta);