* [PATCH BUGFIX ] ipv6: fix the bug of address check
@ 2010-05-17 12:23 Shan Wei
2010-05-17 17:31 ` Stephen Hemminger
0 siblings, 1 reply; 5+ messages in thread
From: Shan Wei @ 2010-05-17 12:23 UTC (permalink / raw)
To: David Miller, Stephen Hemminger; +Cc: netdev@vger.kernel.org
If there are several IPv6 addresses with same hash value in hashlist,
and they are all not matched with addr argument.
In this case, ipv6_chk_addr() should return 0.
This bug is introduced by commit c2e21293c054817c42eb5fa9c613d2ad51954136
(title: ipv6: convert addrconf list to hlist).
Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
net/ipv6/addrconf.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3984f52..d8e5907 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1291,7 +1291,7 @@ int ipv6_chk_addr(struct net *net, struct in6_addr *addr,
}
rcu_read_unlock_bh();
- return ifp != NULL;
+ return node != NULL;
}
EXPORT_SYMBOL(ipv6_chk_addr);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH BUGFIX ] ipv6: fix the bug of address check
2010-05-17 12:23 [PATCH BUGFIX ] ipv6: fix the bug of address check Shan Wei
@ 2010-05-17 17:31 ` Stephen Hemminger
2010-05-18 0:50 ` Shan Wei
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2010-05-17 17:31 UTC (permalink / raw)
To: Shan Wei; +Cc: David Miller, netdev@vger.kernel.org
On Mon, 17 May 2010 20:23:38 +0800
Shan Wei <shanwei@cn.fujitsu.com> wrote:
>
> If there are several IPv6 addresses with same hash value in hashlist,
> and they are all not matched with addr argument.
> In this case, ipv6_chk_addr() should return 0.
>
> This bug is introduced by commit c2e21293c054817c42eb5fa9c613d2ad51954136
> (title: ipv6: convert addrconf list to hlist).
>
> Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
> ---
> net/ipv6/addrconf.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 3984f52..d8e5907 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -1291,7 +1291,7 @@ int ipv6_chk_addr(struct net *net, struct in6_addr *addr,
> }
> rcu_read_unlock_bh();
>
> - return ifp != NULL;
> + return node != NULL;
> }
> EXPORT_SYMBOL(ipv6_chk_addr);
>
Why not this instead. I don't like depending on the value of the
loop variable in the hlist_for_each()
--- a/net/ipv6/addrconf.c 2010-05-17 10:27:58.218628126 -0700
+++ b/net/ipv6/addrconf.c 2010-05-17 10:29:46.012198338 -0700
@@ -1274,7 +1274,7 @@ static int ipv6_count_addresses(struct i
int ipv6_chk_addr(struct net *net, struct in6_addr *addr,
struct net_device *dev, int strict)
{
- struct inet6_ifaddr *ifp = NULL;
+ struct inet6_ifaddr *ifp;
struct hlist_node *node;
unsigned int hash = ipv6_addr_hash(addr);
@@ -1283,15 +1283,16 @@ int ipv6_chk_addr(struct net *net, struc
if (!net_eq(dev_net(ifp->idev->dev), net))
continue;
if (ipv6_addr_equal(&ifp->addr, addr) &&
- !(ifp->flags&IFA_F_TENTATIVE)) {
- if (dev == NULL || ifp->idev->dev == dev ||
- !(ifp->scope&(IFA_LINK|IFA_HOST) || strict))
- break;
+ !(ifp->flags&IFA_F_TENTATIVE) &&
+ (dev == NULL || ifp->idev->dev == dev ||
+ !(ifp->scope&(IFA_LINK|IFA_HOST) || strict))) {
+ rcu_read_unlock_bh();
+ return 1;
}
}
- rcu_read_unlock_bh();
- return ifp != NULL;
+ rcu_read_unlock_bh();
+ return 0;
}
EXPORT_SYMBOL(ipv6_chk_addr);
--
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH BUGFIX ] ipv6: fix the bug of address check
2010-05-17 17:31 ` Stephen Hemminger
@ 2010-05-18 0:50 ` Shan Wei
2010-05-18 1:02 ` [PATCH net-next] " Stephen Hemminger
0 siblings, 1 reply; 5+ messages in thread
From: Shan Wei @ 2010-05-18 0:50 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev@vger.kernel.org
Stephen Hemminger wrote, at 05/18/2010 01:31 AM:
>
> Why not this instead. I don't like depending on the value of the
> loop variable in the hlist_for_each()
>
> --- a/net/ipv6/addrconf.c 2010-05-17 10:27:58.218628126 -0700
> +++ b/net/ipv6/addrconf.c 2010-05-17 10:29:46.012198338 -0700
> @@ -1274,7 +1274,7 @@ static int ipv6_count_addresses(struct i
> int ipv6_chk_addr(struct net *net, struct in6_addr *addr,
> struct net_device *dev, int strict)
> {
> - struct inet6_ifaddr *ifp = NULL;
> + struct inet6_ifaddr *ifp;
> struct hlist_node *node;
> unsigned int hash = ipv6_addr_hash(addr);
>
> @@ -1283,15 +1283,16 @@ int ipv6_chk_addr(struct net *net, struc
> if (!net_eq(dev_net(ifp->idev->dev), net))
> continue;
> if (ipv6_addr_equal(&ifp->addr, addr) &&
> - !(ifp->flags&IFA_F_TENTATIVE)) {
> - if (dev == NULL || ifp->idev->dev == dev ||
> - !(ifp->scope&(IFA_LINK|IFA_HOST) || strict))
> - break;
> + !(ifp->flags&IFA_F_TENTATIVE) &&
> + (dev == NULL || ifp->idev->dev == dev ||
> + !(ifp->scope&(IFA_LINK|IFA_HOST) || strict))) {
> + rcu_read_unlock_bh();
> + return 1;
> }
> }
> - rcu_read_unlock_bh();
>
> - return ifp != NULL;
> + rcu_read_unlock_bh();
> + return 0;
> }
> EXPORT_SYMBOL(ipv6_chk_addr);
>
>
>
This looks good to me.
Can you send a intact patch to David with my Report-by or Review-by ?
--
Best Regards
-----
Shan Wei
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH net-next] ipv6: fix the bug of address check
2010-05-18 0:50 ` Shan Wei
@ 2010-05-18 1:02 ` Stephen Hemminger
2010-05-18 5:27 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2010-05-18 1:02 UTC (permalink / raw)
To: Shan Wei, David Miller; +Cc: netdev@vger.kernel.org
The duplicate address check code got broken in the conversion
to hlist (2.6.35). The earlier patch did not fix the case where
two addresses match same hash value. Use two exit paths,
rather than depending on state of loop variables (from macro).
Based on earlier fix by Shan Wei.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Reviewed-by: Shan Wei <shanwei@cn.fujitsu.com>
--- a/net/ipv6/addrconf.c 2010-05-17 10:27:58.218628126 -0700
+++ b/net/ipv6/addrconf.c 2010-05-17 10:29:46.012198338 -0700
@@ -1274,7 +1274,7 @@ static int ipv6_count_addresses(struct i
int ipv6_chk_addr(struct net *net, struct in6_addr *addr,
struct net_device *dev, int strict)
{
- struct inet6_ifaddr *ifp = NULL;
+ struct inet6_ifaddr *ifp;
struct hlist_node *node;
unsigned int hash = ipv6_addr_hash(addr);
@@ -1283,15 +1283,16 @@ int ipv6_chk_addr(struct net *net, struc
if (!net_eq(dev_net(ifp->idev->dev), net))
continue;
if (ipv6_addr_equal(&ifp->addr, addr) &&
- !(ifp->flags&IFA_F_TENTATIVE)) {
- if (dev == NULL || ifp->idev->dev == dev ||
- !(ifp->scope&(IFA_LINK|IFA_HOST) || strict))
- break;
+ !(ifp->flags&IFA_F_TENTATIVE) &&
+ (dev == NULL || ifp->idev->dev == dev ||
+ !(ifp->scope&(IFA_LINK|IFA_HOST) || strict))) {
+ rcu_read_unlock_bh();
+ return 1;
}
}
- rcu_read_unlock_bh();
- return ifp != NULL;
+ rcu_read_unlock_bh();
+ return 0;
}
EXPORT_SYMBOL(ipv6_chk_addr);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net-next] ipv6: fix the bug of address check
2010-05-18 1:02 ` [PATCH net-next] " Stephen Hemminger
@ 2010-05-18 5:27 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2010-05-18 5:27 UTC (permalink / raw)
To: shemminger; +Cc: shanwei, netdev
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 17 May 2010 18:02:21 -0700
> The duplicate address check code got broken in the conversion
> to hlist (2.6.35). The earlier patch did not fix the case where
> two addresses match same hash value. Use two exit paths,
> rather than depending on state of loop variables (from macro).
>
> Based on earlier fix by Shan Wei.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> Reviewed-by: Shan Wei <shanwei@cn.fujitsu.com>
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-05-18 5:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-17 12:23 [PATCH BUGFIX ] ipv6: fix the bug of address check Shan Wei
2010-05-17 17:31 ` Stephen Hemminger
2010-05-18 0:50 ` Shan Wei
2010-05-18 1:02 ` [PATCH net-next] " Stephen Hemminger
2010-05-18 5:27 ` David Miller
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).