wifi: mac80211: ethtool: always hold wiphy mutex

Commit Message

Johannes Berg Sept. 19, 2023, 6:40 a.m. UTC

From: Johannes Berg <[e-mail]>

Drivers should really be able to rely on the wiphy mutex
being held all the time, unless otherwise documented. For
ethtool, that wasn't quite right. Fix and clarify this in
both code and documentation.

Reported-by: [e-mail]
Fixes: 0e8185ce1dde ("wifi: mac80211: check wiphy mutex in ops")
Signed-off-by: Johannes Berg <[e-mail]>
---
 include/net/mac80211.h | 4 ++++
 net/mac80211/ethtool.c | 9 ++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

kernel test robot Sept. 21, 2023, 4:36 a.m. UTC | #1

Hi Johannes,

kernel test robot noticed the following build errors:

[auto build test ERROR on linux-next/master]
[also build test ERROR on linus/master v6.6-rc2]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Johannes-Berg/wifi-mac80211-ethtool-always-hold-wiphy-mutex/20230919-144127
base:   linux-next/master
patch link:    https://lore.kernel.org/r/20230919084051.942b0bbde0ce.I7215d6cd3bcb4bb8631ddf872356408dd69477fe%40changeid
patch subject: [PATCH] wifi: mac80211: ethtool: always hold wiphy mutex
config: x86_64-randconfig-102-20230921 (https://download.01.org/0day-ci/archive/20230921/[e-mail]/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230921/[e-mail]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[e-mail]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[e-mail]/

All errors (new ones prefixed by >>):

   net/mac80211/ethtool.c: In function 'ieee80211_set_ringparam':
>> net/mac80211/ethtool.c:27:13: error: 'sdata' undeclared (first use in this function); did you mean '_sdata'?
     wiphy_lock(sdata->local->hw.wiphy);
                ^~~~~
                _sdata
   net/mac80211/ethtool.c:27:13: note: each undeclared identifier is reported only once for each function it appears in
   net/mac80211/ethtool.c: In function 'ieee80211_get_ringparam':
   net/mac80211/ethtool.c:43:13: error: 'sdata' undeclared (first use in this function); did you mean '_sdata'?
     wiphy_lock(sdata->local->hw.wiphy);
                ^~~~~
                _sdata


vim +27 net/mac80211/ethtool.c

    15	
    16	static int ieee80211_set_ringparam(struct net_device *dev,
    17					   struct ethtool_ringparam *rp,
    18					   struct kernel_ethtool_ringparam *kernel_rp,
    19					   struct netlink_ext_ack *extack)
    20	{
    21		struct ieee80211_local *local = wiphy_priv(dev->ieee80211_ptr->wiphy);
    22		int ret;
    23	
    24		if (rp->rx_mini_pending != 0 || rp->rx_jumbo_pending != 0)
    25			return -EINVAL;
    26	
  > 27		wiphy_lock(sdata->local->hw.wiphy);
    28		ret = drv_set_ringparam(local, rp->tx_pending, rp->rx_pending);
    29		wiphy_unlock(sdata->local->hw.wiphy);
    30	
    31		return ret;
    32	}
    33

Patch

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 8d993f6ab919..c8bc36821b54 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -4067,11 +4067,15 @@  struct ieee80211_prep_tx_info {
  *	This callback must be atomic.
  *
  * @get_et_sset_count:  Ethtool API to get string-set count.
+ *	Note: the wiphy mutex is not held for this callback since it's
+ *	expected to return a static value.
  *
  * @get_et_stats:  Ethtool API to get a set of u64 stats.
  *
  * @get_et_strings:  Ethtool API to get a set of strings to describe stats
  *	and perhaps other supported types of ethtool data-sets.
+ *	Note: the wiphy mutex is not held for this callback since it's
+ *	expected to return a static value.
  *
  * @mgd_prepare_tx: Prepare for transmitting a management frame for association
  *	before associated. In multi-channel scenarios, a virtual interface is
diff --git a/net/mac80211/ethtool.c b/net/mac80211/ethtool.c
index 9894d2024470..f2e8c2eff25f 100644
--- a/net/mac80211/ethtool.c
+++ b/net/mac80211/ethtool.c
@@ -19,11 +19,16 @@  static int ieee80211_set_ringparam(struct net_device *dev,
 				   struct netlink_ext_ack *extack)
 {
 	struct ieee80211_local *local = wiphy_priv(dev->ieee80211_ptr->wiphy);
+	int ret;
 
 	if (rp->rx_mini_pending != 0 || rp->rx_jumbo_pending != 0)
 		return -EINVAL;
 
-	return drv_set_ringparam(local, rp->tx_pending, rp->rx_pending);
+	wiphy_lock(sdata->local->hw.wiphy);
+	ret = drv_set_ringparam(local, rp->tx_pending, rp->rx_pending);
+	wiphy_unlock(sdata->local->hw.wiphy);
+
+	return ret;
 }
 
 static void ieee80211_get_ringparam(struct net_device *dev,
@@ -35,8 +40,10 @@  static void ieee80211_get_ringparam(struct net_device *dev,
 
 	memset(rp, 0, sizeof(*rp));
 
+	wiphy_lock(sdata->local->hw.wiphy);
 	drv_get_ringparam(local, &rp->tx_pending, &rp->tx_max_pending,
 			  &rp->rx_pending, &rp->rx_max_pending);
+	wiphy_unlock(sdata->local->hw.wiphy);
 }
 
 static const char ieee80211_gstrings_sta_stats[][ETH_GSTRING_LEN] = {