bonding: add ip checks when store ip target
Subsystems (3)
PATCH RESEND 1/1 bonding: add ip checks when store ip target
I met a Bug when I add ip target with the wrong ip address:
echo +500.500.500.500 > /sys/class/net/bond0/bonding/arp_ip_target
the wrong ip address will transfor to 245.245.245.244 and add to the ip target success, it is uncorrect, so I add checks to avoid adding wrong address.
The in4_pton() will set wrong ip address to 0.0.0.0, it will return by the next check and will not add to ip target.
--- drivers/net/bonding/bond_sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/bonding/bond_sysfs.c +1 -1
@@ -653,7 +653,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,int ind, i, j, ret = -EINVAL ; targets = bond->params.arp_targets; - newtarget =in_aton (buf +1 ); +in4_pton (buf +1 ,strlen (buf) -1 , (u8 *)&newtarget,-1 ,NULL );/* look for adds */ if (buf[0 ] =='+' ) {if ((newtarget ==0 ) || (newtarget ==htonl (INADDR_BROADCAST ))) {
Signature · 1 line
1.8.0
Quoted 21 lines · Ding Tianhong
I met a Bug when I add ip target with the wrong ip address:
echo +500.500.500.500 >
/sys/class/net/bond0/bonding/arp_ip_targetthe wrong ip address will transfor to 245.245.245.244 and add to the ip target success, it is uncorrect, so I add checks to avoid adding wrong address.
The
in4_pton()will set wrong ip address to 0.0.0.0, it will return by the next check and will not add to ip target.Signed-off-by Ding Tianhong <[email protected]>---
drivers/net/bonding/bond_sysfs.c| 2 +-1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/bonding/bond_sysfs.c +0
-0
drivers/net/bonding/bond_sysfs.c static ssize_t bonding_store_arp_targets(struct device *d, · line 656
Quoted 5 lines
drivers/net/bonding/bond_sysfs.c +1
-1@@ -653,7 +653,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,int ind, i, j, ret = -EINVAL ; targets = bond->params.arp_targets; - newtarget =in_aton (buf +1 ); +in4_pton (buf +1 ,strlen (buf) -1 , (u8 *)&newtarget,-1 ,NULL );
No need for strlen(buf)-1, if you specify -1 it will compute it by itself.
Also, you might simplify the code a bit in the function. Otherwise - looks good, thank you.
Untested patch:
drivers/net/bonding/bond_sysfs.c +7 -13
@@ -661,16 +661,16 @@ static ssize_t bonding_store_arp_targets(struct device *d,unsigned long *targets_rx;int ind, i, j, ret = -EINVAL ; +if (!in4_pton (buf +1 ,-1 , (u8 *)&newtarget,-1 ,NULL )) { +pr_err ("%s: invalid ARP target specified, use +<addr> or -<addr>\n" , + bond->dev->name); + goto out; + } + targets = bond->params.arp_targets; - newtarget =in_aton (buf +1 ); +/* 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);
@@ -693,12 +693,6 @@ static ssize_t bonding_store_arp_targets(struct device *d, targets[ind] = newtarget;write_unlock_bh (&bond->lock); }else if (buf[0 ] =='-' ) { -if ((newtarget ==0 ) || (newtarget ==htonl (INADDR_BROADCAST ))) { -pr_err ("%s: invalid ARP target %pI4 specified for removal\n" , - bond->dev->name, &newtarget); - goto out; - } - ind =bond_get_targets_ip (targets, newtarget);if (ind ==-1 ) { pr_err("%s: unable to remove nonexistent ARP target %pI4.\n",
Quoted 78 lines · Veaceslav Falico
Quoted 21 lines · Ding Tianhong
I met a Bug when I add ip target with the wrong ip address:
echo +500.500.500.500 >
/sys/class/net/bond0/bonding/arp_ip_targetthe wrong ip address will transfor to 245.245.245.244 and add to the ip target success, it is uncorrect, so I add checks to avoid adding wrong address.
The
in4_pton()will set wrong ip address to 0.0.0.0, it will return by the next check and will not add to ip target.Signed-off-by Ding Tianhong <[email protected]>---
drivers/net/bonding/bond_sysfs.c| 2 +-1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/bonding/bond_sysfs.c +0
-0drivers/net/bonding/bond_sysfs.c static ssize_t bonding_store_arp_targets(struct · line 657
Quoted 6 lines
drivers/net/bonding/bond_sysfs.c +1
-1@@ -653,7 +653,7 @@ static ssize_t bonding_store_arp_targets(structdevice *d, int ind, i, j, ret = -EINVAL; targets = bond->params.arp_targets; - newtarget =in_aton (buf +1 ); +in4_pton (buf +1 ,strlen (buf) -1 , (u8 *)&newtarget,-1 ,NULL );No need for strlen(buf)-1, if you specify -1 it will compute it by itself.
Also, you might simplify the code a bit in the function. Otherwise - looks good, thank you.
Untested patch:
drivers/net/bonding/bond_sysfs.c +7
-7@@ -661,16 +661,16 @@ static ssize_t bonding_store_arp_targets(structdevice *d, unsigned long *targets_rx;int ind, i, j, ret = -EINVAL ; +if (!in4_pton (buf +1 ,-1 , (u8 *)&newtarget,-1 ,NULL )) { +pr_err ("%s: invalid ARP target specified, use +<addr> or -<addr>\n" , + bond->dev->name); + goto out; + } + targets = bond->params.arp_targets; - newtarget =in_aton (buf +1 ); +/* 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);drivers/net/bonding/bond_sysfs.c +0
-6@@ -693,12 +693,6 @@ static ssize_t bonding_store_arp_targets(structdevice *d, targets[ind] = newtarget;write_unlock_bh (&bond->lock); }else if (buf[0 ] =='-' ) { -if ((newtarget ==0 ) || (newtarget ==htonl (INADDR_BROADCAST ))) { -pr_err ("%s: invalid ARP target %pI4 specified for removal\n" , - bond->dev->name, &newtarget); - goto out; - } - ind =bond_get_targets_ip (targets, newtarget);if (ind ==-1 ) {
Quoted 2 lines
pr_err("%s: unable to remove nonexistent ARP target %pI4.\n",
Although there is no substantial changes, but more simple more beautiful, I'll take it, resend it later, thanks for your opinion.
Regards
Ding
Quoted 4 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