netdev - Re: [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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 8 Jul 2008 14:23:53 +0200
From:	"Julius Volz" <juliusv@...gle.com>
To:	"Johannes Berg" <johannes@...solutions.net>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] net/wireless/nl80211.c: fix endless Netlink callback loop.

On Tue, Jul 8, 2008, Johannes Berg wrote:
>> Also, iterations where the filling of an element fails should not be counted as
>> completed, so idx should not be incremented in this case.
>
> Seems to be the case on both points, however,
>
>> --- 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;
>> +             }
>
> I see much of the problem stemming from incrementing 'idx' at the
> beginning of the loop, can't we just move it to the end?

idx still needs to be incremented in the 'continue' case, so that
alone wouldn't help. I'm not sure if there is a way to make this look
more intuitive?

Julius

-- 
Google Switzerland GmbH
--
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