netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* IPv6 duplicate address bugfix
@ 2003-03-30 12:27 Simone Piunno
  2003-03-30 13:08 ` (usagi-users 02296) " YOSHIFUJI Hideaki / 吉藤英明
  2003-03-31 18:23 ` (usagi-users 02296) IPv6 duplicate address bugfix Peter Bieringer
  0 siblings, 2 replies; 9+ messages in thread
From: Simone Piunno @ 2003-03-30 12:27 UTC (permalink / raw)
  To: netdev, usagi-users; +Cc: linux-kernel, ds6-devel


Hi,

When adding an IPv6 address to a given interface, I'm allowed to
add that address multiple time, e.g.:

[root@abulafia root]# ip addr add 3ffe:4242:4242::1 dev eth0
[root@abulafia root]# ip addr add 3ffe:4242:4242::1 dev eth0
[root@abulafia root]# ip addr add 3ffe:4242:4242::1 dev eth0
[root@abulafia root]# ip addr show dev eth0
2: eth0: <BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast qlen 100
    link/ether 00:48:54:1b:25:30 brd ff:ff:ff:ff:ff:ff
    inet6 3ffe:4242:4242::1/128 scope global 
    inet6 3ffe:4242:4242::1/128 scope global 
    inet6 3ffe:4242:4242::1/128 scope global 
    inet6 fe80::248:54ff:fe1b:2530/10 scope link

Apparently, this is not a stability problem, because I'm allowed to 
delete 3 times that address before receving a "not found" error,
but there's no reason to allow multiple instances of the same address 
on the same interface, so this is a bug nonetheless.

Bug is confirmed on:
  - 2.4.20
  - 2.5.66
  - latest -usagi

Following is a patch attempting to fix this bug.
It's for 2.4.20 but sould apply cleanly on 2.5 too.

Credits:
  Chad N. Tindel  - discovered the bug and showed it to me
  Peter Bieringer - confirmed it's a bug
  Mauro Tortonesi - suggested sending a patch to this list.

Regards,
  Simone Piunno


--- net/ipv6/addrconf.c.orig	2003-03-25 21:33:55.000000000 +0100
+++ net/ipv6/addrconf.c	2003-03-30 13:48:23.000000000 +0200
@@ -89,6 +89,8 @@
 static struct inet6_ifaddr		*inet6_addr_lst[IN6_ADDR_HSIZE];
 static rwlock_t	addrconf_hash_lock = RW_LOCK_UNLOCKED;
 
+static spinlock_t addrconf_add_lock = SPIN_LOCK_UNLOCKED;
+
 /* Protects inet6 devices */
 rwlock_t addrconf_lock = RW_LOCK_UNLOCKED;
 
@@ -621,6 +623,24 @@
 	return ifp != NULL;
 }
 
+static struct inet6_ifaddr *
+ipv6_addr_already_present(struct in6_addr *addr, struct net_device *dev)
+{
+	struct inet6_ifaddr *ifp;
+	u8 hash = ipv6_addr_hash(addr);
+	
+	read_lock_bh(&addrconf_hash_lock);
+	for (ifp = inet6_addr_lst[hash]; ifp; ifp = ifp->lst_next) {
+		if (ipv6_addr_cmp(&ifp->addr, addr) == 0 && ifp->idev->dev == dev) {
+			read_unlock_bh(&addrconf_hash_lock);
+			return ifp;
+		}
+	}
+	read_unlock_bh(&addrconf_hash_lock);
+	return NULL;
+}
+
+
 struct inet6_ifaddr * ipv6_get_ifaddr(struct in6_addr *addr, struct net_device *dev)
 {
 	struct inet6_ifaddr * ifp;
@@ -908,7 +928,7 @@
 		return;
 
 ok:
-
+		spin_lock_bh(&addrconf_add_lock);
 		ifp = ipv6_get_ifaddr(&addr, dev);
 
 		if (ifp == NULL && valid_lft) {
@@ -920,12 +940,14 @@
 						    addr_type&IPV6_ADDR_SCOPE_MASK, 0);
 
 			if (ifp == NULL) {
+				spin_unlock_bh(&addrconf_add_lock);
 				in6_dev_put(in6_dev);
 				return;
 			}
 
 			addrconf_dad_start(ifp);
 		}
+		spin_unlock_bh(&addrconf_add_lock);
 
 		if (ifp && valid_lft == 0) {
 			ipv6_del_addr(ifp);
@@ -1033,11 +1055,19 @@
 
 	scope = ipv6_addr_scope(pfx);
 
+	spin_lock_bh(&addrconf_add_lock);
+	if (ipv6_addr_already_present(pfx, dev)) {
+		spin_unlock_bh(&addrconf_add_lock);
+		return -EEXIST;
+	}
+
 	if ((ifp = ipv6_add_addr(idev, pfx, plen, scope, IFA_F_PERMANENT)) != NULL) {
+		spin_unlock_bh(&addrconf_add_lock);
 		addrconf_dad_start(ifp);
 		in6_ifa_put(ifp);
 		return 0;
 	}
+	spin_unlock_bh(&addrconf_add_lock);
 
 	return -ENOBUFS;
 }
-- 
 Simone Piunno -- http://members.ferrara.linux.it/pioppo 
.-------  Adde parvum parvo magnus acervus erit  -------.
 Ferrara Linux Users Group - http://www.ferrara.linux.it 
 Deep Space 6, IPv6 on Linux - http://www.deepspace6.net 
 GNU Mailman, Mailing List Manager - http://www.list.org 
`-------------------------------------------------------'

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: (usagi-users 02296) IPv6 duplicate address bugfix
  2003-03-30 12:27 IPv6 duplicate address bugfix Simone Piunno
@ 2003-03-30 13:08 ` YOSHIFUJI Hideaki / 吉藤英明
  2003-03-30 14:58   ` [PATCH] IPv6: Don't assign a same IPv6 address on a same interface (is Re: IPv6 duplicate address bugfix) YOSHIFUJI Hideaki / 吉藤英明
  2003-03-31 18:23 ` (usagi-users 02296) IPv6 duplicate address bugfix Peter Bieringer
  1 sibling, 1 reply; 9+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2003-03-30 13:08 UTC (permalink / raw)
  To: usagi-users, pioppo; +Cc: netdev, linux-kernel, ds6-devel

In article <20030330122705.GA18283@ferrara.linux.it> (at Sun, 30 Mar 2003 14:27:05 +0200), Simone Piunno <pioppo@ferrara.linux.it> says:

> When adding an IPv6 address to a given interface, I'm allowed to
> add that address multiple time, e.g.:
> 
> [root@abulafia root]# ip addr add 3ffe:4242:4242::1 dev eth0
> [root@abulafia root]# ip addr add 3ffe:4242:4242::1 dev eth0
> [root@abulafia root]# ip addr add 3ffe:4242:4242::1 dev eth0
> [root@abulafia root]# ip addr show dev eth0
> 2: eth0: <BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast qlen 100
>     link/ether 00:48:54:1b:25:30 brd ff:ff:ff:ff:ff:ff
>     inet6 3ffe:4242:4242::1/128 scope global 
>     inet6 3ffe:4242:4242::1/128 scope global 
>     inet6 3ffe:4242:4242::1/128 scope global 
>     inet6 fe80::248:54ff:fe1b:2530/10 scope link
> 
> Apparently, this is not a stability problem, because I'm allowed to 
> delete 3 times that address before receving a "not found" error,
> but there's no reason to allow multiple instances of the same address 
> on the same interface, so this is a bug nonetheless.
> 
> Bug is confirmed on:
>   - 2.4.20
>   - 2.5.66
>   - latest -usagi

usagi code does not act like that.

In my environment,

# ip addr add 3ffe:4242:4242::1 dev eth0
# ip addr add 3ffe:4242:4242::1 dev eth0
RTNETLINK answers: No buffer space available
# ip addr add 3ffe:4242:4242::1 dev eth0
RTNETLINK answers: No buffer space available

And, patch does not seem optimal. I'd take a look at very soon.

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] IPv6: Don't assign a same IPv6 address on a same interface (is Re: IPv6 duplicate address bugfix)
  2003-03-30 13:08 ` (usagi-users 02296) " YOSHIFUJI Hideaki / 吉藤英明
@ 2003-03-30 14:58   ` YOSHIFUJI Hideaki / 吉藤英明
  2003-03-30 16:36     ` Simone Piunno
  0 siblings, 1 reply; 9+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2003-03-30 14:58 UTC (permalink / raw)
  To: davem, kuznet; +Cc: netdev, linux-kernel, usagi, yoshfuji, pioppo

In article <20030330.220829.129728506.yoshfuji@linux-ipv6.org> (at Sun, 30 Mar 2003 22:08:29 +0900 (JST)), YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org> says:

> And, patch does not seem optimal. I'd take a look at very soon.

Here's our patch based on our fix in August, 2001.
Question: should we use spin_lock_bh() instead of spin_lock()?

--------
Don't assign a same IPv6 address on a same interface.
This patch is against linux-2.5.66.
We believe this fix should be suitable on linux-2.4 tree.
(This patch itself conflicts at the first chunk...)

Thanks in advance.

-------------------------------------------------------------------
Patch-Name: Don't assign a same IPv6 address on a same interface
Patch-Id: FIX_2_5_66_ADDRCONF_DUPADDR-20030330
Patch-Author: YOSHIFUJI Hideaki / USAGI Project <yoshfuji@linux-ipv6.org>
Credit: Yuji SEKIYA / USAGI Project <sekiya@linux-ipv6.org>,
	YOSHIFUJI Hideaki / USAGI Project <yoshfuji@linux-ipv6.org>,
	Simone Piunno <pioppo@ferrara.linux.it>
-------------------------------------------------------------------
Index: net/ipv6/addrconf.c
===================================================================
RCS file: /cvsroot/usagi/usagi-backport/linux25/net/ipv6/addrconf.c,v
retrieving revision 1.1.1.9
retrieving revision 1.1.1.9.2.3
diff -u -r1.1.1.9 -r1.1.1.9.2.3
--- net/ipv6/addrconf.c	25 Mar 2003 04:33:45 -0000	1.1.1.9
+++ net/ipv6/addrconf.c	30 Mar 2003 13:50:41 -0000	1.1.1.9.2.3
@@ -30,6 +30,8 @@
  *						address validation timer.
  *	YOSHIFUJI Hideaki @USAGI	:	Privacy Extensions (RFC3041)
  *						support.
+ *	Yuji SEKIYA @USAGI		:	Don't assign a same IPv6
+ *						address on a same interface.
  */
 
 #include <linux/config.h>
@@ -126,6 +128,8 @@
 static void addrconf_rs_timer(unsigned long data);
 static void ipv6_ifa_notify(int event, struct inet6_ifaddr *ifa);
 
+static int ipv6_chk_same_addr(const struct in6_addr *addr, struct net_device *dev);
+
 static struct notifier_block *inet6addr_chain;
 
 struct ipv6_devconf ipv6_devconf =
@@ -492,10 +496,21 @@
 {
 	struct inet6_ifaddr *ifa;
 	int hash;
+	static spinlock_t lock = SPIN_LOCK_UNLOCKED;
+
+	spin_lock(&lock);
+
+	/* Ignore adding duplicate addresses on an interface */
+	if (ipv6_chk_same_addr(addr, idev->dev)) {
+		spin_unlock(&lock);
+		ADBG(("ipv6_add_addr: already assigned\n"));
+		return NULL;
+	}
 
 	ifa = kmalloc(sizeof(struct inet6_ifaddr), GFP_ATOMIC);
 
 	if (ifa == NULL) {
+		spin_unlock(&lock);
 		ADBG(("ipv6_add_addr: malloc failed\n"));
 		return NULL;
 	}
@@ -514,6 +529,7 @@
 	if (idev->dead) {
 		read_unlock(&addrconf_lock);
 		kfree(ifa);
+		spin_unlock(&lock);
 		return NULL;
 	}
 
@@ -551,6 +567,7 @@
 	in6_ifa_hold(ifa);
 	write_unlock_bh(&idev->lock);
 	read_unlock(&addrconf_lock);
+	spin_unlock(&lock);
 
 	notifier_call_chain(&inet6addr_chain,NETDEV_UP,ifa);
 
@@ -921,6 +938,23 @@
 		    !(ifp->flags&IFA_F_TENTATIVE)) {
 			if (dev == NULL || ifp->idev->dev == dev ||
 			    !(ifp->scope&(IFA_LINK|IFA_HOST)))
+				break;
+		}
+	}
+	read_unlock_bh(&addrconf_hash_lock);
+	return ifp != NULL;
+}
+
+static
+int ipv6_chk_same_addr(const struct in6_addr *addr, struct net_device *dev)
+{
+	struct inet6_ifaddr * ifp;
+	u8 hash = ipv6_addr_hash(addr);
+
+	read_lock_bh(&addrconf_hash_lock);
+	for(ifp = inet6_addr_lst[hash]; ifp; ifp=ifp->lst_next) {
+		if (ipv6_addr_cmp(&ifp->addr, addr) == 0) {
+			if (dev != NULL && ifp->idev->dev == dev)
 				break;
 		}
 	}

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] IPv6: Don't assign a same IPv6 address on a same interface (is Re: IPv6 duplicate address bugfix)
  2003-03-30 14:58   ` [PATCH] IPv6: Don't assign a same IPv6 address on a same interface (is Re: IPv6 duplicate address bugfix) YOSHIFUJI Hideaki / 吉藤英明
@ 2003-03-30 16:36     ` Simone Piunno
  2003-03-30 18:35       ` YOSHIFUJI Hideaki / 吉藤英明
  0 siblings, 1 reply; 9+ messages in thread
From: Simone Piunno @ 2003-03-30 16:36 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki / ?$B5HF#1QL@
  Cc: davem, kuznet, netdev, linux-kernel, usagi

On Sun, Mar 30, 2003 at 11:58:09PM +0900, YOSHIFUJI Hideaki wrote:

> > And, patch does not seem optimal. I'd take a look at very soon.
> 
> Here's our patch based on our fix in August, 2001.
> Question: should we use spin_lock_bh() instead of spin_lock()?

Because everywhere else in the file {read,write}_lock_bh() is used 
instead of {read,write}_lock(), so I'm assuming that _bh is required 
but I really don't know why.

Anyway I have some critics over your patch: 

 - locking inside ipv6_add_addr() is simpler and more linear but
   semantically wrong because you're unable to tell the user why his 
   "ip addr add" failed.  E.g. you answer ENOBUFS instead of EEXIST.

 - your ipv6_chk_same_addr() does a useless check for (dev != NULL)

   > +static
   > +int ipv6_chk_same_addr(const struct in6_addr *addr, struct net_device *dev)
   > +{
   > +	struct inet6_ifaddr * ifp;
   > +	u8 hash = ipv6_addr_hash(addr);
   > +
   > +	read_lock_bh(&addrconf_hash_lock);
   > +	for(ifp = inet6_addr_lst[hash]; ifp; ifp=ifp->lst_next) {
   > +		if (ipv6_addr_cmp(&ifp->addr, addr) == 0) {
   > +			if (dev != NULL && ifp->idev->dev == dev)
   >  				break;
   >  		}

   your never "break" if dev == NULL, so you could return 0 before
   even acquiring the lock.

Regards,
  Simone

-- 
 Simone Piunno -- http://members.ferrara.linux.it/pioppo 
.-------  Adde parvum parvo magnus acervus erit  -------.
 Ferrara Linux Users Group - http://www.ferrara.linux.it 
 Deep Space 6, IPv6 on Linux - http://www.deepspace6.net 
 GNU Mailman, Mailing List Manager - http://www.list.org 
`-------------------------------------------------------'

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] IPv6: Don't assign a same IPv6 address on a same interface (is Re: IPv6 duplicate address bugfix)
  2003-03-30 16:36     ` Simone Piunno
@ 2003-03-30 18:35       ` YOSHIFUJI Hideaki / 吉藤英明
  2003-03-31  1:34         ` [PATCH] IPv6: Don't assign a same IPv6 address on a same interface YOSHIFUJI Hideaki / 吉藤英明
  0 siblings, 1 reply; 9+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2003-03-30 18:35 UTC (permalink / raw)
  To: pioppo; +Cc: davem, kuznet, netdev, linux-kernel, usagi

In article <20030330163656.GA18645@ferrara.linux.it> (at Sun, 30 Mar 2003 18:36:56 +0200), Simone Piunno <pioppo@ferrara.linux.it> says:

> Because everywhere else in the file {read,write}_lock_bh() is used 
> instead of {read,write}_lock(), so I'm assuming that _bh is required 
> but I really don't know why.

maybe.

>  - locking inside ipv6_add_addr() is simpler and more linear but
>    semantically wrong because you're unable to tell the user why his 
>    "ip addr add" failed.  E.g. you answer ENOBUFS instead of EEXIST.

We don't want to create duplicate address in any case.
ipv6_add_addr() IS right place.
And, we can return error code by using IS_ERR() etc.
I'll fix this.


>  - your ipv6_chk_same_addr() does a useless check for (dev != NULL)
> 
>    > +static
>    > +int ipv6_chk_same_addr(const struct in6_addr *addr, struct net_device *dev)
>    > +{
>    > +	struct inet6_ifaddr * ifp;
>    > +	u8 hash = ipv6_addr_hash(addr);
>    > +
>    > +	read_lock_bh(&addrconf_hash_lock);
>    > +	for(ifp = inet6_addr_lst[hash]; ifp; ifp=ifp->lst_next) {
>    > +		if (ipv6_addr_cmp(&ifp->addr, addr) == 0) {
>    > +			if (dev != NULL && ifp->idev->dev == dev)
>    >  				break;
>    >  		}
> 
>    your never "break" if dev == NULL, so you could return 0 before
>    even acquiring the lock.

It is not a problem because dev is always non-NULL.
However, it should be dev == NULL || ifp->idev->dev == dev.
Thanks.
(I don't understand what you mean by "you could return 0
before even acquiring the lock.")

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] IPv6: Don't assign a same IPv6 address on a same interface
  2003-03-30 18:35       ` YOSHIFUJI Hideaki / 吉藤英明
@ 2003-03-31  1:34         ` YOSHIFUJI Hideaki / 吉藤英明
  2003-03-31 19:05           ` David S. Miller
  0 siblings, 1 reply; 9+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2003-03-31  1:34 UTC (permalink / raw)
  To: davem, kuznet; +Cc: netdev, linux-kernel, usagi, pioppo

In article <20030331.033524.114862210.yoshfuji@linux-ipv6.org> (at Mon, 31 Mar 2003 03:35:24 +0900 (JST)), YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org> says:

> In article <20030330163656.GA18645@ferrara.linux.it> (at Sun, 30 Mar 2003 18:36:56 +0200), Simone Piunno <pioppo@ferrara.linux.it> says:
> 
> >  - locking inside ipv6_add_addr() is simpler and more linear but
> >    semantically wrong because you're unable to tell the user why his 
> >    "ip addr add" failed.  E.g. you answer ENOBUFS instead of EEXIST.
> 
> We don't want to create duplicate address in any case.
> ipv6_add_addr() IS right place.
> And, we can return error code by using IS_ERR() etc.
> I'll fix this.

Here's the revised patch.
Thank you.

Index: net/ipv6/addrconf.c
===================================================================
RCS file: /cvsroot/usagi/usagi-backport/linux25/net/ipv6/addrconf.c,v
retrieving revision 1.1.1.9
retrieving revision 1.1.1.9.2.6
diff -u -r1.1.1.9 -r1.1.1.9.2.6
--- net/ipv6/addrconf.c	25 Mar 2003 04:33:45 -0000	1.1.1.9
+++ net/ipv6/addrconf.c	30 Mar 2003 18:51:29 -0000	1.1.1.9.2.6
@@ -30,6 +30,8 @@
  *						address validation timer.
  *	YOSHIFUJI Hideaki @USAGI	:	Privacy Extensions (RFC3041)
  *						support.
+ *	Yuji SEKIYA @USAGI		:	Don't assign a same IPv6
+ *						address on a same interface.
  */
 
 #include <linux/config.h>
@@ -126,6 +128,8 @@
 static void addrconf_rs_timer(unsigned long data);
 static void ipv6_ifa_notify(int event, struct inet6_ifaddr *ifa);
 
+static int ipv6_chk_same_addr(const struct in6_addr *addr, struct net_device *dev);
+
 static struct notifier_block *inet6addr_chain;
 
 struct ipv6_devconf ipv6_devconf =
@@ -492,12 +496,23 @@
 {
 	struct inet6_ifaddr *ifa;
 	int hash;
+	static spinlock_t lock = SPIN_LOCK_UNLOCKED;
+
+	spin_lock_bh(&lock);
+
+	/* Ignore adding duplicate addresses on an interface */
+	if (ipv6_chk_same_addr(addr, idev->dev)) {
+		spin_unlock_bh(&lock);
+		ADBG(("ipv6_add_addr: already assigned\n"));
+		return ERR_PTR(-EEXIST);
+	}
 
 	ifa = kmalloc(sizeof(struct inet6_ifaddr), GFP_ATOMIC);
 
 	if (ifa == NULL) {
+		spin_unlock_bh(&lock);
 		ADBG(("ipv6_add_addr: malloc failed\n"));
-		return NULL;
+		return ERR_PTR(-ENOBUFS);
 	}
 
 	memset(ifa, 0, sizeof(struct inet6_ifaddr));
@@ -513,8 +528,9 @@
 	read_lock(&addrconf_lock);
 	if (idev->dead) {
 		read_unlock(&addrconf_lock);
+		spin_unlock_bh(&lock);
 		kfree(ifa);
-		return NULL;
+		return ERR_PTR(-ENODEV);	/*XXX*/
 	}
 
 	inet6_ifa_count++;
@@ -551,6 +567,7 @@
 	in6_ifa_hold(ifa);
 	write_unlock_bh(&idev->lock);
 	read_unlock(&addrconf_lock);
+	spin_unlock_bh(&lock);
 
 	notifier_call_chain(&inet6addr_chain,NETDEV_UP,ifa);
 
@@ -697,7 +714,7 @@
 	ift = ipv6_count_addresses(idev) < IPV6_MAX_ADDRESSES ?
 		ipv6_add_addr(idev, &addr, tmp_plen,
 			      ipv6_addr_type(&addr)&IPV6_ADDR_SCOPE_MASK, IFA_F_TEMPORARY) : 0;
-	if (!ift) {
+	if (IS_ERR(ift)) {
 		in6_dev_put(idev);
 		in6_ifa_put(ifp);
 		printk(KERN_INFO
@@ -928,6 +945,23 @@
 	return ifp != NULL;
 }
 
+static
+int ipv6_chk_same_addr(const struct in6_addr *addr, struct net_device *dev)
+{
+	struct inet6_ifaddr * ifp;
+	u8 hash = ipv6_addr_hash(addr);
+
+	read_lock_bh(&addrconf_hash_lock);
+	for(ifp = inet6_addr_lst[hash]; ifp; ifp=ifp->lst_next) {
+		if (ipv6_addr_cmp(&ifp->addr, addr) == 0) {
+			if (dev == NULL || ifp->idev->dev == dev)
+				break;
+		}
+	}
+	read_unlock_bh(&addrconf_hash_lock);
+	return ifp != NULL;
+}
+
 struct inet6_ifaddr * ipv6_get_ifaddr(struct in6_addr *addr, struct net_device *dev)
 {
 	struct inet6_ifaddr * ifp;
@@ -1344,7 +1378,7 @@
 				ifp = ipv6_add_addr(in6_dev, &addr, pinfo->prefix_len,
 						    addr_type&IPV6_ADDR_SCOPE_MASK, 0);
 
-			if (ifp == NULL) {
+			if (IS_ERR(ifp)) {
 				in6_dev_put(in6_dev);
 				return;
 			}
@@ -1499,13 +1533,14 @@
 
 	scope = ipv6_addr_scope(pfx);
 
-	if ((ifp = ipv6_add_addr(idev, pfx, plen, scope, IFA_F_PERMANENT)) != NULL) {
+	ifp = ipv6_add_addr(idev, pfx, plen, scope, IFA_F_PERMANENT);
+	if (!IS_ERR(ifp)) {
 		addrconf_dad_start(ifp);
 		in6_ifa_put(ifp);
 		return 0;
 	}
 
-	return -ENOBUFS;
+	return PTR_ERR(ifp);
 }
 
 static int inet6_addr_del(int ifindex, struct in6_addr *pfx, int plen)
@@ -1597,7 +1632,7 @@
 
 	if (addr.s6_addr32[3]) {
 		ifp = ipv6_add_addr(idev, &addr, 128, scope, IFA_F_PERMANENT);
-		if (ifp) {
+		if (!IS_ERR(ifp)) {
 			spin_lock_bh(&ifp->lock);
 			ifp->flags &= ~IFA_F_TENTATIVE;
 			spin_unlock_bh(&ifp->lock);
@@ -1633,7 +1668,7 @@
 
 				ifp = ipv6_add_addr(idev, &addr, plen, flag,
 						    IFA_F_PERMANENT);
-				if (ifp) {
+				if (!IS_ERR(ifp)) {
 					spin_lock_bh(&ifp->lock);
 					ifp->flags &= ~IFA_F_TENTATIVE;
 					spin_unlock_bh(&ifp->lock);
@@ -1660,7 +1695,7 @@
 	}
 
 	ifp = ipv6_add_addr(idev, &in6addr_loopback, 128, IFA_HOST, IFA_F_PERMANENT);
-	if (ifp) {
+	if (!IS_ERR(ifp)) {
 		spin_lock_bh(&ifp->lock);
 		ifp->flags &= ~IFA_F_TENTATIVE;
 		spin_unlock_bh(&ifp->lock);
@@ -1674,7 +1709,7 @@
 	struct inet6_ifaddr * ifp;
 
 	ifp = ipv6_add_addr(idev, addr, 64, IFA_LINK, IFA_F_PERMANENT);
-	if (ifp) {
+	if (!IS_ERR(ifp)) {
 		addrconf_dad_start(ifp);
 		in6_ifa_put(ifp);
 	}

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: (usagi-users 02296) IPv6 duplicate address bugfix
  2003-03-30 12:27 IPv6 duplicate address bugfix Simone Piunno
  2003-03-30 13:08 ` (usagi-users 02296) " YOSHIFUJI Hideaki / 吉藤英明
@ 2003-03-31 18:23 ` Peter Bieringer
  2003-03-31 18:56   ` [ds6-devel] " Simone Piunno
  1 sibling, 1 reply; 9+ messages in thread
From: Peter Bieringer @ 2003-03-31 18:23 UTC (permalink / raw)
  To: usagi-users, netdev; +Cc: linux-kernel, ds6-devel

Hi,

just my 2 cents, I already saw, that newest USAGI snapshot include a fix.



--On Sunday, March 30, 2003 02:27:05 PM +0200 Simone Piunno
<pioppo@ferrara.linux.it> wrote:

> When adding an IPv6 address to a given interface, I'm allowed to
> add that address multiple time, e.g.:

...

I didn't dig into any patch and also not into related drafts/RFCs, but one
scenario should be catched I think - or to be discussed:

Scenario:

Address was already added by autoconfiguration on receiving advertisement
(limited lifetime). Now the same address would be added manually (unlimited
lifetime).

What (should) happen?

Mho: manual add is allowed, both addresses need to be listed.

        Peter
-- 
Dr. Peter Bieringer                     http://www.bieringer.de/pb/
GPG/PGP Key 0x958F422D               mailto: pb at bieringer dot de 
Deep Space 6 Co-Founder and Core Member  http://www.deepspace6.net/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [ds6-devel] Re: (usagi-users 02296) IPv6 duplicate address bugfix
  2003-03-31 18:23 ` (usagi-users 02296) IPv6 duplicate address bugfix Peter Bieringer
@ 2003-03-31 18:56   ` Simone Piunno
  0 siblings, 0 replies; 9+ messages in thread
From: Simone Piunno @ 2003-03-31 18:56 UTC (permalink / raw)
  To: Peter Bieringer; +Cc: usagi-users, netdev, ds6-devel, linux-kernel

On Mon, Mar 31, 2003 at 08:23:58PM +0200, Peter Bieringer wrote:

> Address was already added by autoconfiguration on receiving advertisement
> (limited lifetime). Now the same address would be added manually (unlimited
> lifetime).
> 
> What (should) happen?
> 
> Mho: manual add is allowed, both addresses need to be listed.

I'd prefer this variant:

manual add is allowed and overwrites the autoconfigured address.

-- 
 Simone Piunno -- http://members.ferrara.linux.it/pioppo 
.-------  Adde parvum parvo magnus acervus erit  -------.
 Ferrara Linux Users Group - http://www.ferrara.linux.it 
 Deep Space 6, IPv6 on Linux - http://www.deepspace6.net 
 GNU Mailman, Mailing List Manager - http://www.list.org 
`-------------------------------------------------------'

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] IPv6: Don't assign a same IPv6 address on a same interface
  2003-03-31  1:34         ` [PATCH] IPv6: Don't assign a same IPv6 address on a same interface YOSHIFUJI Hideaki / 吉藤英明
@ 2003-03-31 19:05           ` David S. Miller
  0 siblings, 0 replies; 9+ messages in thread
From: David S. Miller @ 2003-03-31 19:05 UTC (permalink / raw)
  To: yoshfuji; +Cc: kuznet, netdev, linux-kernel, usagi, pioppo

   From: YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org>
   Date: Mon, 31 Mar 2003 10:34:51 +0900 (JST)

   In article <20030331.033524.114862210.yoshfuji@linux-ipv6.org> (at Mon, 31 Mar 2003 03:35:24 +0900 (JST)), YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org> says:
   
   > In article <20030330163656.GA18645@ferrara.linux.it> (at Sun, 30 Mar 2003 18:36:56 +0200), Simone Piunno <pioppo@ferrara.linux.it> says:
   > 
   > >  - locking inside ipv6_add_addr() is simpler and more linear but
   > >    semantically wrong because you're unable to tell the user why his 
   > >    "ip addr add" failed.  E.g. you answer ENOBUFS instead of EEXIST.
   > 
   > We don't want to create duplicate address in any case.
   > ipv6_add_addr() IS right place.
   > And, we can return error code by using IS_ERR() etc.
   > I'll fix this.
   
   Here's the revised patch.

Applied to both 2.4.x and 2.5.x.

BTW, 2.4.x patch failed in two spots, one was author comment
which I easily fixed, second was in privacy code which I did not
apply yet to 2.4.x (I fixed this too, don't worry).

I do not want to put privacy code into 2.4.x until crypto is there.
I plan to put crypto lib into 2.4.22-pre1.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2003-03-31 19:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-03-30 12:27 IPv6 duplicate address bugfix Simone Piunno
2003-03-30 13:08 ` (usagi-users 02296) " YOSHIFUJI Hideaki / 吉藤英明
2003-03-30 14:58   ` [PATCH] IPv6: Don't assign a same IPv6 address on a same interface (is Re: IPv6 duplicate address bugfix) YOSHIFUJI Hideaki / 吉藤英明
2003-03-30 16:36     ` Simone Piunno
2003-03-30 18:35       ` YOSHIFUJI Hideaki / 吉藤英明
2003-03-31  1:34         ` [PATCH] IPv6: Don't assign a same IPv6 address on a same interface YOSHIFUJI Hideaki / 吉藤英明
2003-03-31 19:05           ` David S. Miller
2003-03-31 18:23 ` (usagi-users 02296) IPv6 duplicate address bugfix Peter Bieringer
2003-03-31 18:56   ` [ds6-devel] " Simone Piunno

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