netdev - [PATCH] net/wireless/nl80211.c: fix endless Netlink callback loop.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1215518539-19587-1-git-send-email-juliusv@google.com>
Date:	Tue,  8 Jul 2008 14:02:19 +0200
From:	Julius Volz <juliusv@...gle.com>
To:	netdev@...r.kernel.org
Cc:	johannes@...solutions.net, Julius Volz <juliusv@...gle.com>
Subject: [PATCH] net/wireless/nl80211.c: fix endless Netlink callback loop.

Although I only tested similar code (I don't use any of this wireless
code), the state maintainance between Netlink dump callback invocations
seems wrong here and should lead to an endless loop. There are also other
examples in the same file which might have the same problem. Perhaps someone
can actually test this (or refute my logic).

Take the simple example with only one element in the list (which should fit
into the message):

1. invocation:
  Start:
    idx = 0, start = 0
  Loop:
    condition (++idx < start) => (1 < 0) => false
    => no continue, fill one entry, exit loop, return skb->len > 0

2. invocation:
  Start:
    idx = 0, start = 1
  Loop:
    condition (++idx < start) => (1 < 1) => false
    => no continue, fill the same entry again, exit loop, return skb->len > 0

3. invocation:
  Same as 2. invocation, endless invocation of callback.

Also, iterations where the filling of an element fails should not be counted as
completed, so idx should not be incremented in this case.

Signed-off-by: Julius Volz <juliusv@...gle.com>

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fb75f26..b7fefff 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -194,22 +194,24 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
 static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	int idx = 0;
 	int start = cb->args[0];
 	struct cfg80211_registered_device *dev;

 	mutex_lock(&cfg80211_drv_mutex);
 	list_for_each_entry(dev, &cfg80211_drv_list, list) {
-		if (++idx < start)
+		if (++idx <= start)
 			continue;
 		if (nl80211_send_wiphy(skb, NETLINK_CB(cb->skb).pid,
 				       cb->nlh->nlmsg_seq, NLM_F_MULTI,
-				       dev) < 0)
+				       dev) < 0) {
+			idx--;
 			break;
+		}
 	}
 	mutex_unlock(&cfg80211_drv_mutex);

 	cb->args[0] = idx;

 	return skb->len;
 }

-- 
1.5.4.5

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html