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

Message-ID: <20131114101028.GQ19702@redhat.com>
Date:	Thu, 14 Nov 2013 11:10:28 +0100
From:	Veaceslav Falico <vfalico@...hat.com>
To:	Ding Tianhong <dingtianhong@...wei.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 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!

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