3.16.46-rc1 review patch. If anyone has any objections, please let me know. ------------------ From: Johannes Berg <johannes.berg@xxxxxxxxx> 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. Reported-by: Sowmini Varadhan <sowmini.varadhan@xxxxxxxxxx> Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx> Signed-off-by: Johannes Berg <johannes.berg@xxxxxxxxx> [bwh: Backported to 3.16: - Drop changes to nl80211_dump_interface(), nl80211_dump_mpp(), nl80211_prepare_vendor_dump(), nl80211_vendor_cmd_dump() - Adjust context] Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx> --- net/wireless/nl80211.c | 127 ++++++++++++++++++++++--------------------------- 1 file changed, 56 insertions(+), 71 deletions(-) --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -471,21 +471,17 @@ static int nl80211_prepare_wdev_dump(str { 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_rdev((*wdev)->wiphy); /* 0 is the first index - add 1 to parse only once */ cb->args[0] = (*rdev)->wiphy_idx + 1; @@ -495,10 +491,8 @@ static int nl80211_prepare_wdev_dump(str 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_rdev(wiphy); *wdev = NULL; @@ -509,21 +503,11 @@ static int nl80211_prepare_wdev_dump(str } } - 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 */ @@ -3727,9 +3711,10 @@ static int nl80211_dump_station(struct s int sta_idx = cb->args[2]; int err; + rtnl_lock(); err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev); if (err) - return err; + goto out_err; if (!wdev->netdev) { err = -EINVAL; @@ -3765,7 +3750,7 @@ static int nl80211_dump_station(struct s cb->args[2] = sta_idx; err = skb->len; out_err: - nl80211_finish_wdev_dump(rdev); + rtnl_unlock(); return err; } @@ -4443,9 +4428,10 @@ static int nl80211_dump_mpath(struct sk_ int path_idx = cb->args[2]; int err; + rtnl_lock(); err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev); if (err) - return err; + goto out_err; if (!rdev->ops->dump_mpath) { err = -EOPNOTSUPP; @@ -4479,7 +4465,7 @@ static int nl80211_dump_mpath(struct sk_ cb->args[2] = path_idx; err = skb->len; out_err: - nl80211_finish_wdev_dump(rdev); + rtnl_unlock(); return err; } @@ -6157,9 +6143,12 @@ static int nl80211_dump_scan(struct sk_b 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); @@ -6182,7 +6171,7 @@ static int nl80211_dump_scan(struct sk_b wdev_unlock(wdev); cb->args[2] = idx; - nl80211_finish_wdev_dump(rdev); + rtnl_unlock(); return skb->len; } @@ -6255,9 +6244,10 @@ static int nl80211_dump_survey(struct sk int survey_idx = cb->args[2]; int res; + rtnl_lock(); res = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev); if (res) - return res; + goto out_err; if (!wdev->netdev) { res = -EINVAL; @@ -6303,7 +6293,7 @@ static int nl80211_dump_survey(struct sk cb->args[2] = survey_idx; res = skb->len; out_err: - nl80211_finish_wdev_dump(rdev); + rtnl_unlock(); return res; }