[PATCH 0/4] rtnl-locking fixes

[PATCH 0/4] rtnl-locking fixes

[Johannes Berg]

As really I expected, a number of issues were reported just now by
syzbot, and while looking I found one more. Fix them.

[PATCH 1/4] nl80211: call cfg80211_dev_rename() under RTNL

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

This is required, and we have an assertion, move the RTNL
unlock down to cover cfg80211_dev_rename().

Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
Reported-by: syzbot+ed107c5fa3e21cdcd86e@syzkaller.appspotmail.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/nl80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e5e9d889f00f..3b45a9593e71 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3220,7 +3220,6 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 		wdev = netdev->ieee80211_ptr;
 
 	wiphy_lock(&rdev->wiphy);
-	rtnl_unlock();
 
 	/*
 	 * end workaround code, by now the rdev is available
@@ -3230,6 +3229,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 	if (info->attrs[NL80211_ATTR_WIPHY_NAME])
 		result = cfg80211_dev_rename(
 			rdev, nla_data(info->attrs[NL80211_ATTR_WIPHY_NAME]));
+	rtnl_unlock();
 
 	if (result)
 		goto out;
-- 
2.26.2

[PATCH 2/4] wext: call cfg80211_change_iface() with wiphy lock held

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

This is needed now that all the driver callbacks are protected by
the wiphy lock rather than (just) the RTNL.

Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
Reported-by: syzbot+d2d412349f88521938aa@syzkaller.appspotmail.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/wext-compat.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 2e35cb78221e..0c6ea6212496 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -39,6 +39,7 @@ int cfg80211_wext_siwmode(struct net_device *dev, struct iw_request_info *info,
 	struct cfg80211_registered_device *rdev;
 	struct vif_params vifparams;
 	enum nl80211_iftype type;
+	int ret;
 
 	rdev = wiphy_to_rdev(wdev->wiphy);
 
@@ -61,7 +62,11 @@ int cfg80211_wext_siwmode(struct net_device *dev, struct iw_request_info *info,
 
 	memset(&vifparams, 0, sizeof(vifparams));
 
-	return cfg80211_change_iface(rdev, dev, type, &vifparams);
+	wiphy_lock(wdev->wiphy);
+	ret = cfg80211_change_iface(rdev, dev, type, &vifparams);
+	wiphy_unlock(wdev->wiphy);
+
+	return ret;
 }
 EXPORT_WEXT_HANDLER(cfg80211_wext_siwmode);
 
-- 
2.26.2

[PATCH 3/4] wext: call cfg80211_set_encryption() with wiphy lock held

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Similar to the previous commit, we need to hold the wiphy lock
here. There's a second instance that is correct already, fix
this one as well.

Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/wext-compat.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 0c6ea6212496..a8320dc59af7 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -655,6 +655,7 @@ static int cfg80211_wext_siwencodeext(struct net_device *dev,
 	bool remove = false;
 	struct key_params params;
 	u32 cipher;
+	int ret;
 
 	if (wdev->iftype != NL80211_IFTYPE_STATION &&
 	    wdev->iftype != NL80211_IFTYPE_ADHOC)
@@ -726,12 +727,16 @@ static int cfg80211_wext_siwencodeext(struct net_device *dev,
 		params.seq_len = 6;
 	}
 
-	return cfg80211_set_encryption(
+	wiphy_lock(wdev->wiphy);
+	ret = cfg80211_set_encryption(
 			rdev, dev,
 			!(ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY),
 			addr, remove,
 			ext->ext_flags & IW_ENCODE_EXT_SET_TX_KEY,
 			idx, &params);
+	wiphy_unlock(wdev->wiphy);
+
+	return ret;
 }
 
 static int cfg80211_wext_giwencode(struct net_device *dev,
-- 
2.26.2

[PATCH 4/4] cfg80211: call cfg80211_destroy_ifaces() with wiphy lock held

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

This is needed since it calls into the driver, which must have the
same context as if we got to destroy an interface through nl80211.
Fix this, and add a direct lockdep assertion so we don't see it
pop up only when the driver calls back to cfg80211.

Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
Reported-by: syzbot+4305e814f9b267131776@syzkaller.appspotmail.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 200cd9f5fd5f..18f9a5c214b5 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -334,6 +334,7 @@ void cfg80211_destroy_ifaces(struct cfg80211_registered_device *rdev)
 	struct wireless_dev *wdev, *tmp;
 
 	ASSERT_RTNL();
+	lockdep_assert_wiphy(&rdev->wiphy);
 
 	list_for_each_entry_safe(wdev, tmp, &rdev->wiphy.wdev_list, list) {
 		if (wdev->nl_owner_dead)
@@ -349,7 +350,9 @@ static void cfg80211_destroy_iface_wk(struct work_struct *work)
 			    destroy_work);
 
 	rtnl_lock();
+	wiphy_lock(&rdev->wiphy);
 	cfg80211_destroy_ifaces(rdev);
+	wiphy_unlock(&rdev->wiphy);
 	rtnl_unlock();
 }
 
-- 
2.26.2