Commit b4637271 authored by Julius Volz's avatar Julius Volz Committed by John W. Linville

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: default avatarJulius Volz <juliusv@google.com>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent 0e25b4ef
...@@ -199,13 +199,15 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb) ...@@ -199,13 +199,15 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
mutex_lock(&cfg80211_drv_mutex); mutex_lock(&cfg80211_drv_mutex);
list_for_each_entry(dev, &cfg80211_drv_list, list) { list_for_each_entry(dev, &cfg80211_drv_list, list) {
if (++idx < start) if (++idx <= start)
continue; continue;
if (nl80211_send_wiphy(skb, NETLINK_CB(cb->skb).pid, if (nl80211_send_wiphy(skb, NETLINK_CB(cb->skb).pid,
cb->nlh->nlmsg_seq, NLM_F_MULTI, cb->nlh->nlmsg_seq, NLM_F_MULTI,
dev) < 0) dev) < 0) {
idx--;
break; break;
} }
}
mutex_unlock(&cfg80211_drv_mutex); mutex_unlock(&cfg80211_drv_mutex);
cb->args[0] = idx; cb->args[0] = idx;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment