netdev - Re: [PATCH net v2] bonding: add ip checks when store ip target

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-prev] [day] [month] [year] [list]

Message-ID: <5284A494.5020408@huawei.com>
Date:	Thu, 14 Nov 2013 18:23:16 +0800
From:	Ding Tianhong <dingtianhong@...wei.com>
To:	Veaceslav Falico <vfalico@...hat.com>
CC:	Jay Vosburgh <fubar@...ibm.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	"David S. Miller" <davem@...emloft.net>,
	Nikolay Aleksandrov <nikolay@...hat.com>,
	Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net v2] bonding: add ip checks when store ip target

On 2013/11/14 18:10, Veaceslav Falico wrote:
> On Thu, Nov 14, 2013 at 10:35:44AM +0800, Ding Tianhong wrote:
> ...snip...
>> -    newtarget = in_aton(buf + 1);
>> +    if (!in4_pton(buf + 1, -1, (u8 *)&newtarget, -1, NULL) ||
>> +            newtarget == 0) {
> 
> Good catch on that newtarget verification, forgot to add it. However, the
> previous checked not only for all-zeroes, but also for the broadcast, so
> it'd be great if you could maintain the previous functionality/checks.
> 
> Or, even better, you could do something like SCTP does in
> IS_IPV4_UNUSABLE_ADDRESS():
> 
> 353 #define IS_IPV4_UNUSABLE_ADDRESS(a)         \
> 354         ((htonl(INADDR_BROADCAST) == a) ||  \
> 355          ipv4_is_multicast(a) ||            \
> 356          ipv4_is_zeronet(a) ||              \
> 357          ipv4_is_test_198(a) ||             \
> 358          ipv4_is_anycast_6to4(a))
> 
> It's up to you, though, I think that either way works good - it's
> sysadmin's job to know which addresses can/have to be used.
> 
> Also, fix the identation.
> 
> Thanks!
> 

ok

>> +        pr_err("%s: invalid ARP target %pI4 specified for addition\n",
>> +               bond->dev->name, &newtarget);
>> +        goto out;
>> +    }
>>     /* look for adds */
>>     if (buf[0] == '+') {
>> -        if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) {
>> -            pr_err("%s: invalid ARP target %pI4 specified for addition\n",
>> -                   bond->dev->name, &newtarget);
>> -            goto out;
>> -        }
>> -
>>         if (bond_get_targets_ip(targets, newtarget) != -1) { /* dup */
>>             pr_err("%s: ARP target %pI4 is already present\n",
>>                    bond->dev->name, &newtarget);
> ...snip...
> 
> .
> 


--
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