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

Commit 3a5a423b authored by Johannes Berg's avatar Johannes Berg
Browse files

nl80211: fix attrbuf access race by allocating a separate one 



Since my commit 3713b4e3 ("nl80211: allow splitting wiphy
information in dumps"), nl80211_dump_wiphy() uses the global
nl80211_fam.attrbuf for parsing the incoming data. This wouldn't
be a problem if it only did so on the first dump iteration which
is locked against other commands in generic netlink, but due to
space constraints in cb->args (the needed state doesn't fit) I
decided to always parse the original message. That's racy though
since nl80211_fam.attrbuf could be used by some other parsing in
generic netlink concurrently.

For now, fix this by allocating a separate parse buffer (it's a
bit too big for the stack, currently 1448 bytes on 64-bit). For
-next, I'll change the code to parse into the global buffer in
the first round only and then allocate a smaller buffer to keep
the data in cb->args.

Reported-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
Acked-by: default avatarDavid S. Miller <davem@davemloft.net>
Acked-by: default avatarJohn W. Linville <linville@tuxdriver.com>
Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
parent 795d855d

net/wireless/nl80211.c

+9 −2
Original line number Original line Diff line number Diff line
@@ -1564,12 +1564,17 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb) 
	struct cfg80211_registered_device *dev;

	struct cfg80211_registered_device *dev;

	s64 filter_wiphy = -1;

	s64 filter_wiphy = -1;

	bool split = false;

	bool split = false;

	struct nlattr **tb = nl80211_fam.attrbuf;

	struct nlattr **tb;

	int res;

	int res;





	/* will be zeroed in nlmsg_parse() */

	tb = kmalloc(sizeof(*tb) * (NL80211_ATTR_MAX + 1), GFP_KERNEL);

	if (!tb)

		return -ENOMEM;



	mutex_lock(&cfg80211_mutex);

	mutex_lock(&cfg80211_mutex);

	res = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,

	res = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,

			  tb, nl80211_fam.maxattr, nl80211_policy);

			  tb, NL80211_ATTR_MAX, nl80211_policy);

	if (res == 0) {

	if (res == 0) {

		split = tb[NL80211_ATTR_SPLIT_WIPHY_DUMP];

		split = tb[NL80211_ATTR_SPLIT_WIPHY_DUMP];

		if (tb[NL80211_ATTR_WIPHY])

		if (tb[NL80211_ATTR_WIPHY])
@@ -1583,6 +1588,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb) 
			netdev = dev_get_by_index(sock_net(skb->sk), ifidx);

			netdev = dev_get_by_index(sock_net(skb->sk), ifidx);

			if (!netdev) {

			if (!netdev) {

				mutex_unlock(&cfg80211_mutex);

				mutex_unlock(&cfg80211_mutex);

				kfree(tb);

				return -ENODEV;

				return -ENODEV;

			}

			}

			if (netdev->ieee80211_ptr) {

			if (netdev->ieee80211_ptr) {
@@ -1593,6 +1599,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb) 
			dev_put(netdev);

			dev_put(netdev);

		}

		}

	}

	}

	kfree(tb);





	list_for_each_entry(dev, &cfg80211_rdev_list, list) {

	list_for_each_entry(dev, &cfg80211_rdev_list, list) {

		if (!net_eq(wiphy_net(&dev->wiphy), sock_net(skb->sk)))

		if (!net_eq(wiphy_net(&dev->wiphy), sock_net(skb->sk)))