2023-11-15 13:08:28

by Johannes Berg

Subject: [PATCH] wifi: cfg80211: hold wiphy mutex for send_interface

From: Johannes Berg <[email protected]>

Given all the locking rework in mac80211, we pretty much
need to get into the driver with the wiphy mutex held in
all callbacks. This is already mostly the case, but as
Johan reported, in the get_txpower it may not be true.

Lock the wiphy mutex around nl80211_send_iface(), then
is also around callers of nl80211_notify_iface(). This
is easy to do, fixes the problem, and aligns the locking
between various calls to it in different parts of the
code of cfg80211.

Fixes: 0e8185ce1dde ("wifi: mac80211: check wiphy mutex in ops")
Reported-by: Johan Hovold <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]
Signed-off-by: Johannes Berg <[email protected]>
---
net/wireless/core.c | 4 ++--
net/wireless/nl80211.c | 5 +++++
2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 758c9a2a12c0..1786ebc5e5cd 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -191,13 +191,13 @@ int cfg80211_switch_netns(struct cfg80211_registered_device *rdev,
return err;
}

+ wiphy_lock(&rdev->wiphy);
list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
if (!wdev->netdev)
continue;
nl80211_notify_iface(rdev, wdev, NL80211_CMD_DEL_INTERFACE);
}

- wiphy_lock(&rdev->wiphy);
nl80211_notify_wiphy(rdev, NL80211_CMD_DEL_WIPHY);

wiphy_net_set(&rdev->wiphy, net);
@@ -206,13 +206,13 @@ int cfg80211_switch_netns(struct cfg80211_registered_device *rdev,
WARN_ON(err);

nl80211_notify_wiphy(rdev, NL80211_CMD_NEW_WIPHY);
- wiphy_unlock(&rdev->wiphy);

list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
if (!wdev->netdev)
continue;
nl80211_notify_iface(rdev, wdev, NL80211_CMD_NEW_INTERFACE);
}
+ wiphy_unlock(&rdev->wiphy);

return 0;
}
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 569234bc2be6..358ca88b5292 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3822,6 +3822,8 @@ static int nl80211_send_iface(struct sk_buff *msg, u32 portid, u32 seq, int flag
struct net_device *dev = wdev->netdev;
void *hdr;

+ lockdep_assert_wiphy(&rdev->wiphy);
+
WARN_ON(cmd != NL80211_CMD_NEW_INTERFACE &&
cmd != NL80211_CMD_DEL_INTERFACE &&
cmd != NL80211_CMD_SET_INTERFACE);
@@ -3989,6 +3991,7 @@ static int nl80211_dump_interface(struct sk_buff *skb, struct netlink_callback *

if_idx = 0;

+ wiphy_lock(&rdev->wiphy);
list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
if (if_idx < if_start) {
if_idx++;
@@ -3998,10 +4001,12 @@ static int nl80211_dump_interface(struct sk_buff *skb, struct netlink_callback *
cb->nlh->nlmsg_seq, NLM_F_MULTI,
rdev, wdev,
NL80211_CMD_NEW_INTERFACE) < 0) {
+ wiphy_unlock(&rdev->wiphy);
goto out;
}
if_idx++;
}
+ wiphy_unlock(&rdev->wiphy);

wp_idx++;
}
--
2.41.0

2023-11-15 16:00:37

by Johan Hovold

Subject: Re: [PATCH] wifi: cfg80211: hold wiphy mutex for send_interface

On Wed, Nov 15, 2023 at 01:06:16PM +0100, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> Given all the locking rework in mac80211, we pretty much
> need to get into the driver with the wiphy mutex held in
> all callbacks. This is already mostly the case, but as
> Johan reported, in the get_txpower it may not be true.
>
> Lock the wiphy mutex around nl80211_send_iface(), then
> is also around callers of nl80211_notify_iface(). This
> is easy to do, fixes the problem, and aligns the locking
> between various calls to it in different parts of the
> code of cfg80211.
>
> Fixes: 0e8185ce1dde ("wifi: mac80211: check wiphy mutex in ops")
> Reported-by: Johan Hovold <[email protected]>
> Closes: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Johannes Berg <[email protected]>

Thanks for the quick fix. With this I no longer see any lockdep splat on
boot with the X13s:

Tested-by: Johan Hovold <[email protected]>

Johan