* [PATCH net RESEND] bonding: add ip checks when store ip target @ 2013-11-13 2:19 Ding Tianhong 2013-11-13 10:00 ` Veaceslav Falico 0 siblings, 1 reply; 3+ messages in thread From: Ding Tianhong @ 2013-11-13 2:19 UTC (permalink / raw) To: Jay Vosburgh, Andy Gospodarek, David S. Miller, Nikolay Aleksandrov, Veaceslav Falico, Netdev 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. Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> --- drivers/net/bonding/bond_sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index ec9b646..e0c97fb 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -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))) { -- 1.8.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net RESEND] bonding: add ip checks when store ip target 2013-11-13 2:19 [PATCH net RESEND] bonding: add ip checks when store ip target Ding Tianhong @ 2013-11-13 10:00 ` Veaceslav Falico 2013-11-13 14:35 ` Ding Tianhong 0 siblings, 1 reply; 3+ messages in thread From: Veaceslav Falico @ 2013-11-13 10:00 UTC (permalink / raw) To: Ding Tianhong Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller, Nikolay Aleksandrov, Netdev On Wed, Nov 13, 2013 at 10:19:02AM +0800, Ding Tianhong wrote: >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. > >Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> >--- > drivers/net/bonding/bond_sysfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c >index ec9b646..e0c97fb 100644 >--- a/drivers/net/bonding/bond_sysfs.c >+++ b/drivers/net/bonding/bond_sysfs.c >@@ -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: diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index c29b836..e3fff6e 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -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", ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net RESEND] bonding: add ip checks when store ip target 2013-11-13 10:00 ` Veaceslav Falico @ 2013-11-13 14:35 ` Ding Tianhong 0 siblings, 0 replies; 3+ messages in thread From: Ding Tianhong @ 2013-11-13 14:35 UTC (permalink / raw) To: Veaceslav Falico Cc: Ding Tianhong, Jay Vosburgh, Andy Gospodarek, David S. Miller, Nikolay Aleksandrov, Netdev 于 2013/11/13 18:00, Veaceslav Falico 写道: > On Wed, Nov 13, 2013 at 10:19:02AM +0800, Ding Tianhong wrote: >> 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. >> >> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> >> --- >> drivers/net/bonding/bond_sysfs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/bonding/bond_sysfs.c >> b/drivers/net/bonding/bond_sysfs.c >> index ec9b646..e0c97fb 100644 >> --- a/drivers/net/bonding/bond_sysfs.c >> +++ b/drivers/net/bonding/bond_sysfs.c >> @@ -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: > > diff --git a/drivers/net/bonding/bond_sysfs.c > b/drivers/net/bonding/bond_sysfs.c > index c29b836..e3fff6e 100644 > --- a/drivers/net/bonding/bond_sysfs.c > +++ b/drivers/net/bonding/bond_sysfs.c > @@ -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", > -- Although there is no substantial changes, but more simple more beautiful, I'll take it, resend it later, thanks for your opinion. Regards Ding > 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 > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-11-13 14:47 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-13 2:19 [PATCH net RESEND] bonding: add ip checks when store ip target Ding Tianhong 2013-11-13 10:00 ` Veaceslav Falico 2013-11-13 14:35 ` Ding Tianhong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).