Re: [IPV6] Fix refcnt leak in addrconf regen timer

[Herbert Xu <herbert@gondor.apana.org.au>]

On Wed, Jul 09, 2003 at 10:18:59PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> 
> How about this?

Then you're going to get nasty messages about the timer being added
twice when try_regen is run.

> @@ -368,6 +368,7 @@
>  				dev, dev->name);
>  			ndev->cnf.use_tempaddr = -1;
>  		} else {
> +			in6_dev_hold(ndev);
>  			__ipv6_regen_rndid(ndev);
>  		}

I missed this hold in my previous patch.

So here is my latest update :)
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.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

Index: kernel-source-2.5/net/ipv6/addrconf.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/ipv6/addrconf.c,v
retrieving revision 1.1.1.10
diff -u -r1.1.1.10 addrconf.c
--- kernel-source-2.5/net/ipv6/addrconf.c	2 Jul 2003 20:56:02 -0000	1.1.1.10
+++ kernel-source-2.5/net/ipv6/addrconf.c	9 Jul 2003 21:18:10 -0000
@@ -368,7 +368,8 @@
 				dev, dev->name);
 			ndev->cnf.use_tempaddr = -1;
 		} else {
-			__ipv6_regen_rndid(ndev);
+			in6_dev_hold(ndev);
+			ipv6_regen_rndid((unsigned long) ndev);
 		}
 #endif
 
@@ -1122,9 +1123,6 @@
 	sg[1].offset = ((long) eui64 & ~PAGE_MASK);
 	sg[1].length = 8;
 
-	if (!del_timer(&idev->regen_timer))
-		in6_dev_hold(idev);
-
 	dev = idev->dev;
 
 	if (ipv6_generate_eui64(eui64, dev)) {
@@ -1137,7 +1135,6 @@
 	spin_lock(&md5_tfm_lock);
 	if (unlikely(md5_tfm == NULL)) {
 		spin_unlock(&md5_tfm_lock);
-		in6_dev_put(idev);
 		return -1;
 	}
 	crypto_digest_init(md5_tfm);
@@ -1170,31 +1167,40 @@
 		if ((idev->rndid[2]|idev->rndid[3]|idev->rndid[4]|idev->rndid[5]|idev->rndid[6]|idev->rndid[7]) == 0x00)
 			goto regen;
 	}
-	
-	idev->regen_timer.expires = jiffies +
-					idev->cnf.temp_prefered_lft * HZ - 
-					idev->cnf.regen_max_retry * idev->cnf.dad_transmits * idev->nd_parms->retrans_time - desync_factor;
-	if (time_before(idev->regen_timer.expires, jiffies)) {
-		idev->regen_timer.expires = 0;
-		printk(KERN_WARNING
-			"__ipv6_regen_rndid(): too short regeneration interval; timer disabled for %s.\n",
-			idev->dev->name);
-		in6_dev_put(idev);
-		return -1;
-	}
 
-	add_timer(&idev->regen_timer);
 	return 0;
 }
 
 static void ipv6_regen_rndid(unsigned long data)
 {
 	struct inet6_dev *idev = (struct inet6_dev *) data;
+	unsigned long expires;
 
 	read_lock_bh(&addrconf_lock);
 	write_lock_bh(&idev->lock);
-	if (!idev->dead)
-		__ipv6_regen_rndid(idev);
+
+	if (idev->dead)
+		goto out;
+
+	if (__ipv6_regen_rndid(idev) < 0)
+		goto out;
+	
+	expires = jiffies +
+		idev->cnf.temp_prefered_lft * HZ - 
+		idev->cnf.regen_max_retry * idev->cnf.dad_transmits * idev->nd_parms->retrans_time - desync_factor;
+	if (time_before(expires, jiffies)) {
+		printk(KERN_WARNING
+			"__ipv6_regen_rndid(): too short regeneration interval; timer disabled for %s.\n",
+			idev->dev->name);
+		goto out;
+	}
+
+	if (!mod_timer(&idev->regen_timer, expires))
+		in6_dev_hold(idev);
+
+out:
+	in6_dev_put(idev);
+
 	write_unlock_bh(&idev->lock);
 	read_unlock_bh(&addrconf_lock);
 }
@@ -1907,6 +1913,27 @@
 	/* Step 3: clear address list */
 
 	write_lock_bh(&idev->lock);
+#ifdef CONFIG_IPV6_PRIVACY
+	if (how == 1 && del_timer(&idev->regen_timer))
+		in6_dev_put(idev);
+
+	/* clear tempaddr list */
+	while ((ifa = idev->tempaddr_list) != NULL) {
+		idev->tempaddr_list = ifa->tmp_next;
+		ifa->tmp_next = NULL;
+		ifa->dead = 1;
+		write_unlock_bh(&idev->lock);
+		spin_lock_bh(&ifa->lock);
+
+		if (ifa->ifpub) {
+			in6_ifa_put(ifa->ifpub);
+			ifa->ifpub = NULL;
+		}
+		spin_unlock_bh(&ifa->lock);
+		in6_ifa_put(ifa);
+		write_lock_bh(&idev->lock);
+	}
+#endif
 	while ((ifa = idev->addr_list) != NULL) {
 		idev->addr_list = ifa->if_next;
 		ifa->if_next = NULL;