Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 56769e7a authored by Johannes Berg's avatar Johannes Berg Committed by Greg Kroah-Hartman
Browse files

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.

Reported-by: default avatarSowmini Varadhan <sowmini.varadhan@oracle.com>
Reported-by: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 7b3c8b2a

net/wireless/nl80211.c

+60 −73
Original line number Diff line number Diff line
@@ -548,21 +548,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_rdev((*wdev)->wiphy);

		/* 0 is the first index - add 1 to parse only once */

		cb->args[0] = (*rdev)->wiphy_idx + 1;
@@ -572,10 +568,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_rdev(wiphy);

		*wdev = NULL;
@@ -586,21 +580,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 */
@@ -2584,17 +2568,17 @@ static int nl80211_dump_interface(struct sk_buff *skb, struct netlink_callback * 
	int filter_wiphy = -1;

	struct cfg80211_registered_device *rdev;

	struct wireless_dev *wdev;

	int ret;



	rtnl_lock();

	if (!cb->args[2]) {

		struct nl80211_dump_wiphy_state state = {

			.filter_wiphy = -1,

		};

		int ret;



		ret = nl80211_dump_wiphy_parse(skb, cb, &state);

		if (ret)

			return ret;

			goto out_unlock;



		filter_wiphy = state.filter_wiphy;
@@ -2639,12 +2623,14 @@ static int nl80211_dump_interface(struct sk_buff *skb, struct netlink_callback * 
		wp_idx++;

	}

 out:

	rtnl_unlock();



	cb->args[0] = wp_idx;

	cb->args[1] = if_idx;



	return skb->len;

	ret = skb->len;

 out_unlock:

	rtnl_unlock();



	return ret;

}



static int nl80211_get_interface(struct sk_buff *skb, struct genl_info *info)
@@ -4371,9 +4357,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, &rdev, &wdev);

	if (err)

		return err;

		goto out_err;



	if (!wdev->netdev) {

		err = -EINVAL;
@@ -4408,7 +4395,7 @@ static int nl80211_dump_station(struct sk_buff *skb, 
	cb->args[2] = sta_idx;

	err = skb->len;

 out_err:

	nl80211_finish_wdev_dump(rdev);

	rtnl_unlock();



	return err;

}
@@ -5179,9 +5166,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, &rdev, &wdev);

	if (err)

		return err;

		goto out_err;



	if (!rdev->ops->dump_mpath) {

		err = -EOPNOTSUPP;
@@ -5214,7 +5202,7 @@ static int nl80211_dump_mpath(struct sk_buff *skb, 
	cb->args[2] = path_idx;

	err = skb->len;

 out_err:

	nl80211_finish_wdev_dump(rdev);

	rtnl_unlock();

	return err;

}
@@ -5374,9 +5362,10 @@ static int nl80211_dump_mpp(struct sk_buff *skb, 
	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_mpp) {

		err = -EOPNOTSUPP;
@@ -5409,7 +5398,7 @@ static int nl80211_dump_mpp(struct sk_buff *skb, 
	cb->args[2] = path_idx;

	err = skb->len;

 out_err:

	nl80211_finish_wdev_dump(rdev);

	rtnl_unlock();

	return err;

}
@@ -7556,9 +7545,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);
@@ -7581,7 +7573,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;

}
@@ -7665,9 +7657,10 @@ static int nl80211_dump_survey(struct sk_buff *skb, struct netlink_callback *cb) 
	int res;

	bool radio_stats;



	rtnl_lock();

	res = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev);

	if (res)

		return res;

		goto out_err;



	/* prepare_wdev_dump parsed the attributes */

	radio_stats = nl80211_fam.attrbuf[NL80211_ATTR_SURVEY_RADIO_STATS];
@@ -7708,7 +7701,7 @@ static int nl80211_dump_survey(struct sk_buff *skb, struct netlink_callback *cb) 
	cb->args[2] = survey_idx;

	res = skb->len;

 out_err:

	nl80211_finish_wdev_dump(rdev);

	rtnl_unlock();

	return res;

}
@@ -11299,17 +11292,13 @@ static int nl80211_prepare_vendor_dump(struct sk_buff *skb, 
	void *data = NULL;

	unsigned int data_len = 0;



	rtnl_lock();



	if (cb->args[0]) {

		/* subtract the 1 again here */

		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;
@@ -11330,13 +11319,11 @@ static int nl80211_prepare_vendor_dump(struct sk_buff *skb, 
			  nl80211_fam.attrbuf, nl80211_fam.maxattr,

			  nl80211_policy);

	if (err)

		goto out_unlock;

		return err;



	if (!nl80211_fam.attrbuf[NL80211_ATTR_VENDOR_ID] ||

	    !nl80211_fam.attrbuf[NL80211_ATTR_VENDOR_SUBCMD]) {

		err = -EINVAL;

		goto out_unlock;

	}

	    !nl80211_fam.attrbuf[NL80211_ATTR_VENDOR_SUBCMD])

		return -EINVAL;



	*wdev = __cfg80211_wdev_from_attrs(sock_net(skb->sk),

					   nl80211_fam.attrbuf);
@@ -11345,10 +11332,8 @@ static int nl80211_prepare_vendor_dump(struct sk_buff *skb, 


	*rdev = __cfg80211_rdev_from_attrs(sock_net(skb->sk),

					   nl80211_fam.attrbuf);

	if (IS_ERR(*rdev)) {

		err = PTR_ERR(*rdev);

		goto out_unlock;

	}

	if (IS_ERR(*rdev))

		return PTR_ERR(*rdev);



	vid = nla_get_u32(nl80211_fam.attrbuf[NL80211_ATTR_VENDOR_ID]);

	subcmd = nla_get_u32(nl80211_fam.attrbuf[NL80211_ATTR_VENDOR_SUBCMD]);
@@ -11361,19 +11346,15 @@ static int nl80211_prepare_vendor_dump(struct sk_buff *skb, 
		if (vcmd->info.vendor_id != vid || vcmd->info.subcmd != subcmd)

			continue;



		if (!vcmd->dumpit) {

			err = -EOPNOTSUPP;

			goto out_unlock;

		}

		if (!vcmd->dumpit)

			return -EOPNOTSUPP;



		vcmd_idx = i;

		break;

	}



	if (vcmd_idx < 0) {

		err = -EOPNOTSUPP;

		goto out_unlock;

	}

	if (vcmd_idx < 0)

		return -EOPNOTSUPP;



	if (nl80211_fam.attrbuf[NL80211_ATTR_VENDOR_DATA]) {

		data = nla_data(nl80211_fam.attrbuf[NL80211_ATTR_VENDOR_DATA]);
@@ -11390,9 +11371,6 @@ static int nl80211_prepare_vendor_dump(struct sk_buff *skb, 


	/* keep rtnl locked in successful case */

	return 0;

 out_unlock:

	rtnl_unlock();

	return err;

}



static int nl80211_vendor_cmd_dump(struct sk_buff *skb,
@@ -11407,9 +11385,10 @@ static int nl80211_vendor_cmd_dump(struct sk_buff *skb, 
	int err;

	struct nlattr *vendor_data;



	rtnl_lock();

	err = nl80211_prepare_vendor_dump(skb, cb, &rdev, &wdev);

	if (err)

		return err;

		goto out;



	vcmd_idx = cb->args[2];

	data = (void *)cb->args[3];
@@ -11418,18 +11397,26 @@ static int nl80211_vendor_cmd_dump(struct sk_buff *skb, 


	if (vcmd->flags & (WIPHY_VENDOR_CMD_NEED_WDEV |

			   WIPHY_VENDOR_CMD_NEED_NETDEV)) {

		if (!wdev)

			return -EINVAL;

		if (!wdev) {

			err = -EINVAL;

			goto out;

		}

		if (vcmd->flags & WIPHY_VENDOR_CMD_NEED_NETDEV &&

		    !wdev->netdev)

			return -EINVAL;

		    !wdev->netdev) {

			err = -EINVAL;

			goto out;

		}



		if (vcmd->flags & WIPHY_VENDOR_CMD_NEED_RUNNING) {

			if (wdev->netdev &&

			    !netif_running(wdev->netdev))

				return -ENETDOWN;

			if (!wdev->netdev && !wdev->p2p_started)

				return -ENETDOWN;

			    !netif_running(wdev->netdev)) {

				err = -ENETDOWN;

				goto out;

			}

			if (!wdev->netdev && !wdev->p2p_started) {

				err = -ENETDOWN;

				goto out;

			}

		}

	}