[PATCH] bonding: enhance the IP address check of arp_ip_target

[PATCH] bonding: enhance the IP address check of arp_ip_target

[Tetsuo Takata]

Hi,


I found this in drivers/net/bonding/bond_main.c.
> /* 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 <takatatt@intellilink.co.jp>

--- linux-2.6.17.3/drivers/net/bonding/bond_main.c	2006-07-01 02:37:38.000000000 +0900
+++ linux-2.6.17.3-bonding-ipcheck/drivers/net/bonding/bond_main.c	2006-07-12 09:51:12.000000000 +0900
@@ -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",

Re: [PATCH] bonding: enhance the IP address check of arp_ip_target

[Stephen Hemminger]

On Wed, 12 Jul 2006 13:25:52 +0900
Tetsuo Takata <takata.tetsuo@oss.ntt.co.jp> wrote:

> Hi,
> 
> 
> I found this in drivers/net/bonding/bond_main.c.
> > /* 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 <takatatt@intellilink.co.jp>
> 
> --- linux-2.6.17.3/drivers/net/bonding/bond_main.c	2006-07-01 02:37:38.000000000 +0900
> +++ linux-2.6.17.3-bonding-ipcheck/drivers/net/bonding/bond_main.c	2006-07-12 09:51:12.000000000 +0900
> @@ -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",
> 
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Why not just use sscanf?

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

Re: [PATCH] bonding: enhance the IP address check of arp_ip_target

[Herbert Xu]

Stephen Hemminger <shemminger@osdl.org> wrote:
> 
> Why not just use sscanf?

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

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] bonding: enhance the IP address check of arp_ip_target

[Tetsuo Takata]

Hi,


Thank you for the comments.


>> Why not just use sscanf?
Here's a fixed version of the previous patch, that uses sscanf.

> 
> Better yet, use a better interface like netlink rather than module
> parameters.
Wouldn't that be overkill?


best regards,

---
Signed-off-by: Tetsuo Takata <takatatt@intellilink.co.jp>

--- linux-2.6.17.4/drivers/net/bonding/bond_main.c	2006-06-18 10:49:35.000000000 +0900
+++ linux-2.6.17.4-enhance-ipcheck/drivers/net/bonding/bond_main.c	2006-07-12 17:20:39.000000000 +0900
@@ -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",