nl80211: fix dumpit error path RTNL deadlocks

commit ea90e0dc8cecba6359b481e24d9c37160f6f524f upstream.

Sowmini pointed out Dmitry's RTNL deadlock report to me, and it turns out
to be perfectly accurate - there are various error paths that miss unlock
of the RTNL.

To fix those, change the locking a bit to not be conditional in all those
nl80211_prepare_*_dump() functions, but make those require the RTNL to
start with, and fix the buggy error paths. This also let me use sparse
(by appropriately overriding the rtnl_lock/rtnl_unlock functions) to
validate the changes.

[js] no mpp and vendor dumps in 3.12 yet

Reported-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index cda142009426c6be16003b6219e147ebe62d790c..bb03e47bf88767187f431060904ab19123e113ed 100644 (file)
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -438,21 +438,17 @@ static int nl80211_prepare_wdev_dump(struct sk_buff *skb,
 {
        int err;
 
-       rtnl_lock();
-
        if (!cb->args[0]) {
                err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
                                  nl80211_fam.attrbuf, nl80211_fam.maxattr,
                                  nl80211_policy);
                if (err)
-                       goto out_unlock;
+                       return err;
 
                *wdev = __cfg80211_wdev_from_attrs(sock_net(skb->sk),
                                                   nl80211_fam.attrbuf);
-               if (IS_ERR(*wdev)) {
-                       err = PTR_ERR(*wdev);
-                       goto out_unlock;
-               }
+               if (IS_ERR(*wdev))
+                       return PTR_ERR(*wdev);
                *rdev = wiphy_to_dev((*wdev)->wiphy);
                /* 0 is the first index - add 1 to parse only once */
                cb->args[0] = (*rdev)->wiphy_idx + 1;
@@ -462,10 +458,8 @@ static int nl80211_prepare_wdev_dump(struct sk_buff *skb,
                struct wiphy *wiphy = wiphy_idx_to_wiphy(cb->args[0] - 1);
                struct wireless_dev *tmp;
 
-               if (!wiphy) {
-                       err = -ENODEV;
-                       goto out_unlock;
-               }
+               if (!wiphy)
+                       return -ENODEV;
                *rdev = wiphy_to_dev(wiphy);
                *wdev = NULL;
 
@@ -476,21 +470,11 @@ static int nl80211_prepare_wdev_dump(struct sk_buff *skb,
                        }
                }
 
-               if (!*wdev) {
-                       err = -ENODEV;
-                       goto out_unlock;
-               }
+               if (!*wdev)
+                       return -ENODEV;
        }
 
        return 0;
- out_unlock:
-       rtnl_unlock();
-       return err;
-}
-
-static void nl80211_finish_wdev_dump(struct cfg80211_registered_device *rdev)
-{
-       rtnl_unlock();
 }
 
 /* IE validation */
@@ -3607,9 +3591,10 @@ static int nl80211_dump_station(struct sk_buff *skb,
        int sta_idx = cb->args[2];
        int err;
 
+       rtnl_lock();
        err = nl80211_prepare_wdev_dump(skb, cb, &dev, &wdev);
        if (err)
-               return err;
+               goto out_err;
 
        if (!wdev->netdev) {
                err = -EINVAL;
@@ -3645,7 +3630,7 @@ static int nl80211_dump_station(struct sk_buff *skb,
        cb->args[2] = sta_idx;
        err = skb->len;
  out_err:
-       nl80211_finish_wdev_dump(dev);
+       rtnl_unlock();
 
        return err;
 }
@@ -4273,9 +4258,10 @@ static int nl80211_dump_mpath(struct sk_buff *skb,
        int path_idx = cb->args[2];
        int err;
 
+       rtnl_lock();
        err = nl80211_prepare_wdev_dump(skb, cb, &dev, &wdev);
        if (err)
-               return err;
+               goto out_err;
 
        if (!dev->ops->dump_mpath) {
                err = -EOPNOTSUPP;
@@ -4309,7 +4295,7 @@ static int nl80211_dump_mpath(struct sk_buff *skb,
        cb->args[2] = path_idx;
        err = skb->len;
  out_err:
-       nl80211_finish_wdev_dump(dev);
+       rtnl_unlock();
        return err;
 }
 
@@ -5853,9 +5839,12 @@ static int nl80211_dump_scan(struct sk_buff *skb, struct netlink_callback *cb)
        int start = cb->args[2], idx = 0;
        int err;
 
+       rtnl_lock();
        err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev);
-       if (err)
+       if (err) {
+               rtnl_unlock();
                return err;
+       }
 
        wdev_lock(wdev);
        spin_lock_bh(&rdev->bss_lock);
@@ -5878,7 +5867,7 @@ static int nl80211_dump_scan(struct sk_buff *skb, struct netlink_callback *cb)
        wdev_unlock(wdev);
 
        cb->args[2] = idx;
-       nl80211_finish_wdev_dump(rdev);
+       rtnl_unlock();
 
        return skb->len;
 }
@@ -5951,9 +5940,10 @@ static int nl80211_dump_survey(struct sk_buff *skb,
        int survey_idx = cb->args[2];
        int res;
 
+       rtnl_lock();
        res = nl80211_prepare_wdev_dump(skb, cb, &dev, &wdev);
        if (res)
-               return res;
+               goto out_err;
 
        if (!wdev->netdev) {
                res = -EINVAL;
@@ -5999,7 +5989,7 @@ static int nl80211_dump_survey(struct sk_buff *skb,
        cb->args[2] = survey_idx;
        res = skb->len;
  out_err:
-       nl80211_finish_wdev_dump(dev);
+       rtnl_unlock();
        return res;
 }