diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 7de6dbdae930..16f6b96e0d88 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -7226,6 +7227,9 @@ enum ieee80211_ap_reg_power { * @netdev: (private) Used to reference back to the netdev, may be %NULL * @identifier: (private) Identifier used in nl80211 to identify this * wireless device if it has no netdev + * @refcnt: (private) references held by nl80211 if there's no netdev, + * to ensure cfg80211_unregister_wdev() cannot return while + * unlocked ops are running * @u: union containing data specific to @iftype * @connected: indicates if connected or not (STA mode) * @wext: (private) Used by the internal wireless extensions compat code @@ -7291,6 +7295,7 @@ struct wireless_dev { struct net_device *netdev; u32 identifier; + refcount_t refcnt; struct list_head mgmt_registrations; u8 mgmt_registrations_need_update:1; @@ -10174,6 +10179,9 @@ u32 cfg80211_calculate_bitrate(struct rate_info *rate); * when the driver wishes to unregister the wdev, e.g. when the hardware device * is unbound from the driver. * + * Note that unlocked operations can run concurrently until this returns, so be + * careful about ordering. + * * Context: Requires the RTNL and wiphy mutex to be held. */ void cfg80211_unregister_wdev(struct wireless_dev *wdev); diff --git a/net/wireless/core.c b/net/wireless/core.c index 01b077909179..777f48a01101 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -1514,6 +1514,16 @@ static void _cfg80211_unregister_wdev(struct wireless_dev *wdev, list_del_rcu(&wdev->list); synchronize_net(); + + /* + * After synchronize_net() any lookups under RCU either have + * references (must wdev_hold() inside the critical section) + * or can't find this wdev anymore. Wait for any wdev_hold() + * users on a pure wdev to get to dev_put(). + */ + if (!wdev->netdev && !refcount_dec_and_test(&wdev->refcnt)) + wait_var_event(&wdev->refcnt, !refcount_read(&wdev->refcnt)); + rdev->devlist_generation++; wiphy_work_cancel(wdev->wiphy, &wdev->disconnect_wk); @@ -1700,6 +1710,7 @@ EXPORT_SYMBOL(cfg80211_stop_link); void cfg80211_init_wdev(struct wireless_dev *wdev) { + refcount_set(&wdev->refcnt, 1); INIT_LIST_HEAD(&wdev->event_list); spin_lock_init(&wdev->event_lock); INIT_LIST_HEAD(&wdev->mgmt_registrations); diff --git a/net/wireless/core.h b/net/wireless/core.h index dfcb9ed5035b..220544a6bb83 100644 --- a/net/wireless/core.h +++ b/net/wireless/core.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include "reg.h" @@ -259,6 +260,26 @@ void cfg80211_init_wdev(struct wireless_dev *wdev); void cfg80211_register_wdev(struct cfg80211_registered_device *rdev, struct wireless_dev *wdev); +static inline void wdev_hold(struct wireless_dev *wdev) +{ + if (wdev->netdev) { + dev_hold(wdev->netdev); + return; + } + + /* also not RTNL, but can't check that */ + lockdep_assert_not_held(&wdev->wiphy->mtx); + refcount_inc(&wdev->refcnt); +} + +static inline void wdev_put(struct wireless_dev *wdev) +{ + if (wdev->netdev) + dev_put(wdev->netdev); + else if (refcount_dec_and_test(&wdev->refcnt)) + wake_up_var(&wdev->refcnt); +} + static inline bool cfg80211_has_monitors_only(struct cfg80211_registered_device *rdev) { lockdep_assert_held(&rdev->wiphy.mtx); diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 6387a312b35e..eb91937a7238 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -266,6 +266,34 @@ nl80211_lock_and_recheck(struct cfg80211_registered_device *rdev, return ERR_PTR(-ENODEV); } +/* + * Must have wdev_hold(), acquire the wiphy mutex. This has to + * look up the wdev again (otherwise it could deadlock against + * wdev removal), so it can return an error pointer. + */ +static struct wireless_dev * +nl80211_lock_held_wdev(struct cfg80211_registered_device *rdev, + struct genl_info *info, struct wireless_dev *wdev) +{ + u32 wdev_id = wdev->identifier; + + /* the wdev reference was keeping the wiphy alive */ + get_device(&rdev->wiphy.dev); + info->user_ptr[1] = NULL; + wdev_put(wdev); + + wdev = nl80211_lock_and_recheck(rdev, genl_info_net(info), wdev_id); + if (IS_ERR(wdev)) + return wdev; + + if (!wdev_running(wdev)) { + wiphy_unlock(&rdev->wiphy); + return ERR_PTR(-ENETDOWN); + } + + return wdev; +} + static int validate_beacon_head(const struct nlattr *attr, struct netlink_ext_ack *extack) { @@ -19906,14 +19934,10 @@ nl80211_epcs_cfg(struct sk_buff *skb, struct genl_info *info) /* * Note: a - * SELECTOR(..., - * NL80211_FLAG_NO_WIPHY_MTX | - * NL80211_FLAG_NEED_WDEV) - * or * SELECTOR(..., NL80211_FLAG_NO_WIPHY_MTX) * - * isn't valid - nothing would ensure the device stays around, - * need at least a netdev, the wiphy mutex, or RTNL. + * isn't valid - nothing would ensure the device stays around, need + * at least a wdev/netdev reference, the wiphy mutex, or RTNL. */ enum nl80211_internal_flags_selector { @@ -19928,6 +19952,13 @@ static u32 nl80211_internal_flags[] = { #undef SELECTOR }; +/* without RTNL or wiphy mutex, a wdev reference keeps everything alive */ +static bool nl80211_need_wdev_ref(u32 internal_flags) +{ + return !(internal_flags & NL80211_FLAG_NEED_RTNL) && + internal_flags & NL80211_FLAG_NO_WIPHY_MTX; +} + static int nl80211_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb, struct genl_info *info) @@ -19936,7 +19967,7 @@ static int nl80211_pre_doit(const struct genl_split_ops *ops, struct net *netns = genl_info_net(info); struct wireless_dev *wdev = NULL; struct net_device *dev = NULL; - bool need_rtnl, locked = false; + bool need_rtnl, wdev_ref, locked = false; u32 internal_flags, wdev_id = 0; int err; @@ -19945,6 +19976,7 @@ static int nl80211_pre_doit(const struct genl_split_ops *ops, internal_flags = nl80211_internal_flags[ops->internal_flags]; need_rtnl = internal_flags & NL80211_FLAG_NEED_RTNL; + wdev_ref = nl80211_need_wdev_ref(internal_flags); if (need_rtnl) rtnl_lock(); @@ -19966,7 +19998,16 @@ static int nl80211_pre_doit(const struct genl_split_ops *ops, } dev = wdev->netdev; - dev_hold(dev); + /* + * Take the wdev ref only if we need it (wdev_ref is true when + * the op is unlocked), we must not try to acquire this with + * a wdev pointer protected by locking, it'd deadlock against + * unregistration. + */ + if (wdev_ref) + wdev_hold(wdev); + else + dev_hold(dev); rdev = wiphy_to_rdev(wdev->wiphy); } @@ -19999,12 +20040,12 @@ static int nl80211_pre_doit(const struct genl_split_ops *ops, locked = true; } else if (rdev && !need_rtnl) { /* - * Without any mutex need a netdev to keep it alive, see - * also the note about selectors - the WARN can't happen + * Without any mutex need a wdev reference to keep it alive, + * see also the note about selectors - the WARN can't happen * without wrong selectors and those don't exist. */ put_device(&rdev->wiphy.dev); - if (WARN_ON(!dev)) { + if (WARN_ON(!wdev)) { err = -EINVAL; goto out; } @@ -20069,7 +20110,10 @@ static int nl80211_pre_doit(const struct genl_split_ops *ops, out: if (locked) wiphy_unlock(&rdev->wiphy); - dev_put(dev); + if (wdev_ref && wdev) + wdev_put(wdev); + else + dev_put(dev); if (need_rtnl) rtnl_unlock(); return err; @@ -20086,17 +20130,23 @@ static void nl80211_post_doit(const struct genl_split_ops *ops, struct genl_info *info) { u32 internal_flags = nl80211_internal_flags[ops->internal_flags]; + struct wireless_dev *wdev = NULL; + struct net_device *dev = NULL; if (info->user_ptr[1]) { if (internal_flags & NL80211_FLAG_NEED_WDEV) { - struct wireless_dev *wdev = info->user_ptr[1]; - - dev_put(wdev->netdev); + wdev = info->user_ptr[1]; + dev = wdev->netdev; } else { - dev_put(info->user_ptr[1]); + dev = info->user_ptr[1]; } } + if (wdev && nl80211_need_wdev_ref(internal_flags)) + wdev_put(wdev); + else + dev_put(dev); + if (info->user_ptr[0] && !(internal_flags & NL80211_FLAG_NO_WIPHY_MTX)) { struct cfg80211_registered_device *rdev = info->user_ptr[0];