Skip to contentSkip to conversation

bonding: enhance the IP address check of arp_ip_target

PATCH In review Tetsuo Takata 3 participants 4 messages Jul 12, 2006

Subsystems (3)
Networking Network drivers Bonding driver
lore
Share & tools
lore

Tetsuo Takata

author
#0 lore raw

PATCH 1/1 bonding: enhance the IP address check of arp_ip_target

Hi,

I found this in drivers/net/bonding/bond_main.c.

Quoted 2 lines

/* not complete check, but should be good enough to
catch mistakes */

I made a patch which I believe is little bit better than this, I hope...

best regards,

---

Signed-off-by Tetsuo Takata <[email protected]>
drivers/net/bonding/bond_main.c +107 -1
@@ -4455,7 +4455,113 @@ static int bond_check_params(struct bond
 	     arp_ip_count++) {
 		/* not complete check, but should be good enough to
 		   catch mistakes */
-		if (!isdigit(arp_ip_target[arp_ip_count][0])) {
+		int i, notip = 0;
+		char *cp;
+
+		cp = arp_ip_target[arp_ip_count];
+
+		/* notip's number is the error code for debug purpose */
+		do {
+			if (cp == NULL) {
+				notip = 1;
+				break;
+			}
+
+			/* check digit */
+			for (i = 0; isdigit(*cp); i++) {
+				if ((i < 0) || (i >= 3)) {
+					notip = 2;
+					break;
+				}
+				cp++;
+			}
+			if (notip)
+				break;
+
+			if (i == 0) {
+				notip = 3;
+				break;
+			}
+
+			/* check delimiter */
+			if (*cp != '.') {
+				notip = 4;
+				break;
+			}
+			cp++;
+
+			/* check digit */
+			for (i = 0; isdigit(*cp); i++) {
+				if ((i < 0) || (i >= 3)) {
+					notip = 5;
+					break;
+				}
+				cp++;
+			}
+			if (notip)
+				break;
+
+			if (i == 0) {
+				notip = 6;
+				break;
+			}
+
+			/* check delimiter */
+			if (*cp != '.') {
+				notip = 7;
+				break;
+			}
+			cp++;
+
+			/* check digit */
+			for (i = 0; isdigit(*cp); i++) {
+				if ((i < 0) || (i >= 3)) {
+					notip = 8;
+					break;
+				}
+				cp++;
+			}
+			if (notip)
+				break;
+
+			if (i == 0) {
+				notip = 9;
+				break;
+			}
+
+			/* check delimiter */
+			if (*cp != '.') {
+				notip = 10;
+				break;
+			}
+			cp++;
+
+			/* check digit */
+			for (i = 0; isdigit(*cp); i++) {
+				if ((i < 0) || (i >= 3)) {
+					notip = 11;
+					break;
+				}
+				cp++;
+			}
+			if (notip)
+				break;
+
+			if (i == 0) {
+				notip = 12;
+				break;
+			}
+
+
+			/* check EOS */
+			if (*cp != '\0') {
+				notip = 13;
+				break;
+			}
+
+		} while(0);
+
+		if (notip) {
 			printk(KERN_WARNING DRV_NAME
 			       ": Warning: bad arp_ip_target module parameter "
 			       "(%s), ARP monitoring will not be performed\n",
#1 lore raw
Quoted 135 lines · On Wed, 12 Jul 2006 13:25:52 +0900 Tetsuo Takata <[email protected]>…

Hi,

I found this in drivers/net/bonding/bond_main.c.

Quoted 2 lines

/* not complete check, but should be good enough to
catch mistakes */

I made a patch which I believe is little bit better than this, I hope...

best regards,

---

Signed-off-by Tetsuo Takata <[email protected]>
drivers/net/bonding/bond_main.c +107 -2
@@ -4455,7 +4455,113 @@ static int bond_check_params(struct bond
 	     arp_ip_count++) {
 		/* not complete check, but should be good enough to
 		   catch mistakes */
-		if (!isdigit(arp_ip_target[arp_ip_count][0])) {
+		int i, notip = 0;
+		char *cp;
+
+		cp = arp_ip_target[arp_ip_count];
+
+		/* notip's number is the error code for debug purpose */
+		do {
+			if (cp == NULL) {
+				notip = 1;
+				break;
+			}
+
+			/* check digit */
+			for (i = 0; isdigit(*cp); i++) {
+				if ((i < 0) || (i >= 3)) {
+					notip = 2;
+					break;
+				}
+				cp++;
+			}
+			if (notip)
+				break;
+
+			if (i == 0) {
+				notip = 3;
+				break;
+			}
+
+			/* check delimiter */
+			if (*cp != '.') {
+				notip = 4;
+				break;
+			}
+			cp++;
+
+			/* check digit */
+			for (i = 0; isdigit(*cp); i++) {
+				if ((i < 0) || (i >= 3)) {
+					notip = 5;
+					break;
+				}
+				cp++;
+			}
+			if (notip)
+				break;
+
+			if (i == 0) {
+				notip = 6;
+				break;
+			}
+
+			/* check delimiter */
+			if (*cp != '.') {
+				notip = 7;
+				break;
+			}
+			cp++;
+
+			/* check digit */
+			for (i = 0; isdigit(*cp); i++) {
+				if ((i < 0) || (i >= 3)) {
+					notip = 8;
+					break;
+				}
+				cp++;
+			}
+			if (notip)
+				break;
+
+			if (i == 0) {
+				notip = 9;
+				break;
+			}
+
+			/* check delimiter */
+			if (*cp != '.') {
+				notip = 10;
+				break;
+			}
+			cp++;
+
+			/* check digit */
+			for (i = 0; isdigit(*cp); i++) {
+				if ((i < 0) || (i >= 3)) {
+					notip = 11;
+					break;
+				}
+				cp++;
+			}
+			if (notip)
+				break;
+
+			if (i == 0) {
+				notip = 12;
+				break;
+			}
+
+
+			/* check EOS */
+			if (*cp != '\0') {
+				notip = 13;
+				break;
+			}
+
+		} while(0);
+
+		if (notip) {
 			printk(KERN_WARNING DRV_NAME
 			       ": Warning: bad arp_ip_target module parameter "
 			       "(%s), ARP monitoring will not be performed\n",
 
-
Quoted 3 lines

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

Why not just use sscanf?

Signature · 2 lines
If one would give me six lines written by the hand of the most honest
man, I would find something in them to have him hanged. -- Cardinal Richlieu
#2 lore raw
Quoted 2 lines · Stephen Hemminger

Why not just use sscanf?

Better yet, use a better interface like netlink rather than module parameters.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
author
#3 lore raw

Hi,

Thank you for the comments.

Quoted 1 line
Quoted 1 line

Why not just use sscanf?

Here's a fixed version of the previous patch, that uses sscanf.

Quoted 3 lines

Better yet, use a better interface like netlink rather than module parameters.

Wouldn't that be overkill?

best regards,

---

Signed-off-by Tetsuo Takata <[email protected]>
drivers/net/bonding/bond_main.c +30 -1
@@ -4455,7 +4455,36 @@ static int bond_check_params(struct bond
 	     arp_ip_count++) {
 		/* not complete check, but should be good enough to
 		   catch mistakes */
-		if (!isdigit(arp_ip_target[arp_ip_count][0])) {
+		int ip1, ip2, ip3, ip4, notip = 0;
+		char dummy;
+
+		/* notip's number means error code for debug purpose */
+		do {
+			if (sscanf(arp_ip_target[arp_ip_count], "%d.%d.%d.%d%c",
+					 	&ip1, &ip2, &ip3, &ip4, &dummy) != 4) {
+				notip = 1;
+				break;
+			} else {
+				if (ip1 < 0 || ip1 > 255) {
+					notip = 1;
+					break;
+				}
+				if (ip2 < 0 || ip2 > 255) {
+					notip = 1;
+					break;
+				}
+				if (ip3 < 0 || ip3 > 255) {
+					notip = 1;
+					break;
+				}
+				if (ip4 < 0 || ip4 > 255) {
+					notip = 1;
+					break;
+				}
+			}
+		} while(0);
+
+		if (notip) {
 			printk(KERN_WARNING DRV_NAME
 			       ": Warning: bad arp_ip_target module parameter "
 			       "(%s), ARP monitoring will not be performed\n",