From: Johannes Berg <[email protected]>
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: [email protected]
Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
Signed-off-by: Johannes Berg <[email protected]>
---
include/net/cfg80211.h | 3 +++
net/wireless/core.c | 4 +++-
2 files changed, 6 insertions(+), 1 deletion(-)
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);
--
2.26.2
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 <[email protected]>
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/[email protected]