cfg80211: fix netdev registration deadlock

Commit Message

Johannes Berg Feb. 1, 2021, 2:04 p.m. UTC

From: Johannes Berg <[e-mail]>

If register_netdevice() fails after having called cfg80211's
netdev notifier (cfg80211_netdev_notifier_call) it will call
the notifier again with UNREGISTER. This would then lock the
wiphy mutex because we're marked as registered, which causes
a deadlock.

Fix this by separately keeping track of whether or not we're
in the middle of registering to also skip the notifier call
on this unregister.

Reported-by: [e-mail]
Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
Signed-off-by: Johannes Berg <[e-mail]>
---
 include/net/cfg80211.h | 3 +++
 net/wireless/core.c    | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

kernel test robot Feb. 1, 2021, 4:56 p.m. UTC | #1

Hi Johannes,

I love your patch! Yet something to improve:

[auto build test ERROR on mac80211-next/master]
[also build test ERROR on linux-review/chaitanya-mgit-gmail-com/cfg80211-Skip-key-deletion-for-open-associations/20210128-170146 next-20210125]
[cannot apply to mac80211/master]
[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/cfg80211-fix-netdev-registration-deadlock/20210201-220922
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: x86_64-randconfig-a016-20210201 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 275c6af7d7f1ed63a03d05b4484413e447133269)
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
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/20b73d4cb250fb38ad81967abcb0776fc3103dc3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Johannes-Berg/cfg80211-fix-netdev-registration-deadlock/20210201-220922
        git checkout 20b73d4cb250fb38ad81967abcb0776fc3103dc3
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

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

All errors (new ones prefixed by >>):

>> net/wireless/core.c:1349:8: error: no member named 'registering' in 'struct wireless_dev'; did you mean 'registered'?
           wdev->registering = true;
                 ^~~~~~~~~~~
                 registered
   include/net/cfg80211.h:5388:30: note: 'registered' declared here
           bool use_4addr, is_running, registered;
                                       ^
   net/wireless/core.c:1365:8: error: no member named 'registering' in 'struct wireless_dev'; did you mean 'registered'?
           wdev->registering = false;
                 ^~~~~~~~~~~
                 registered
   include/net/cfg80211.h:5388:30: note: 'registered' declared here
           bool use_4addr, is_running, registered;
                                       ^
   net/wireless/core.c:1408:34: error: no member named 'registering' in 'struct wireless_dev'; did you mean 'registered'?
                   if (wdev->registered && !wdev->registering) {
                                                  ^~~~~~~~~~~
                                                  registered
   include/net/cfg80211.h:5388:30: note: 'registered' declared here
           bool use_4addr, is_running, registered;
                                       ^
   3 errors generated.


vim +1349 net/wireless/core.c

  1331	
  1332	int cfg80211_register_netdevice(struct net_device *dev)
  1333	{
  1334		struct wireless_dev *wdev = dev->ieee80211_ptr;
  1335		struct cfg80211_registered_device *rdev;
  1336		int ret;
  1337	
  1338		ASSERT_RTNL();
  1339	
  1340		if (WARN_ON(!wdev))
  1341			return -EINVAL;
  1342	
  1343		rdev = wiphy_to_rdev(wdev->wiphy);
  1344	
  1345		lockdep_assert_held(&rdev->wiphy.mtx);
  1346	
  1347		/* we'll take care of this */
  1348		wdev->registered = true;
> 1349		wdev->registering = true;
  1350		ret = register_netdevice(dev);
  1351		if (ret)
  1352			goto out;
  1353	
  1354		if (sysfs_create_link(&dev->dev.kobj, &rdev->wiphy.dev.kobj,
  1355				      "phy80211")) {
  1356			pr_err("failed to add phy80211 symlink to netdev!\n");
  1357			unregister_netdevice(dev);
  1358			ret = -EINVAL;
  1359			goto out;
  1360		}
  1361	
  1362		cfg80211_register_wdev(rdev, wdev);
  1363		ret = 0;
  1364	out:
  1365		wdev->registering = false;
  1366		if (ret)
  1367			wdev->registered = false;
  1368		return ret;
  1369	}
  1370	EXPORT_SYMBOL(cfg80211_register_netdevice);
  1371	

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

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 4741d71ead21..ceeeb1d7cfe5 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4788,6 +4788,8 @@  struct wiphy_iftype_akm_suites {
  *	the same number of arbitrary MAC addresses.
  * @registered: protects ->resume and ->suspend sysfs callbacks against
  *	unregister hardware
+ * @registering: indicates we're doing registration under wiphy lock
+ *	for the notifier
  * @debugfsdir: debugfs directory used for this wiphy (ieee80211/<wiphyname>).
  *	It will be renamed automatically on wiphy renames
  * @dev: (virtual) struct device for this wiphy. The item in
@@ -5033,6 +5035,7 @@  struct wiphy {
 	struct device dev;
 
 	bool registered;
+	bool registering;
 
 	struct dentry *debugfsdir;
 
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 18f9a5c214b5..a2785379df6e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1346,6 +1346,7 @@  int cfg80211_register_netdevice(struct net_device *dev)
 
 	/* we'll take care of this */
 	wdev->registered = true;
+	wdev->registering = true;
 	ret = register_netdevice(dev);
 	if (ret)
 		goto out;
@@ -1361,6 +1362,7 @@  int cfg80211_register_netdevice(struct net_device *dev)
 	cfg80211_register_wdev(rdev, wdev);
 	ret = 0;
 out:
+	wdev->registering = false;
 	if (ret)
 		wdev->registered = false;
 	return ret;
@@ -1403,7 +1405,7 @@  static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		 * It is possible to get NETDEV_UNREGISTER multiple times,
 		 * so check wdev->registered.
 		 */
-		if (wdev->registered) {
+		if (wdev->registered && !wdev->registering) {
 			wiphy_lock(&rdev->wiphy);
 			_cfg80211_unregister_wdev(wdev, false);
 			wiphy_unlock(&rdev->wiphy);