[PATCH 1/2][IPV6] addrconf: fix timer deleting on exit

[PATCH 1/2][IPV6] addrconf: fix timer deleting on exit

[Jarek Poplawski]

It looks like a timer function can be running and rearm
the timer after removing a ipv6 module.


Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>

---

diff -Nurp 2.6.22-rc7-/net/ipv6/addrconf.c 2.6.22-rc7/net/ipv6/addrconf.c
--- 2.6.22-rc7-/net/ipv6/addrconf.c	2007-07-02 09:03:29.000000000 +0200
+++ 2.6.22-rc7/net/ipv6/addrconf.c	2007-07-05 12:27:22.000000000 +0200
@@ -2957,8 +2957,11 @@ restart:
 		read_unlock(&addrconf_hash_lock);
 	}
 
-	addr_chk_timer.expires = time_before(next, jiffies + HZ) ? jiffies + HZ : next;
-	add_timer(&addr_chk_timer);
+	if (ipv6_dev_notf.notifier_call) {	/* exit time? */
+		addr_chk_timer.expires = time_before(next, jiffies + HZ)
+							? jiffies + HZ : next;
+		add_timer(&addr_chk_timer);
+	}
 	spin_unlock_bh(&addrconf_verify_lock);
 }
 
@@ -4281,9 +4284,13 @@ void __exit addrconf_cleanup(void)
 			 */
 		}
 	}
+
+	/* now exit flag for a timer... */
+	ipv6_dev_notf.notifier_call = NULL;
 	write_unlock_bh(&addrconf_hash_lock);
 
-	del_timer(&addr_chk_timer);
+	if (!del_timer_sync(&addr_chk_timer))
+		del_timer_sync(&addr_chk_timer);
 
 	rtnl_unlock();
 

[PATCH 2/2][IPV6] addrconf: fix addrconf_del_timer locking etc.

[Jarek Poplawski]

addrconf_del_timer() is sometimes done without a lock.
IMHO it could be racy e.g. when between del_timer() and
__in6_ifa_put() some other in6_ifa_put() is done.

addrconf_dad_kick() also runs unlocked in one place. 

BTW, I changed a bit one printk to be more precise, I hope.

PS: this patch was prepared on 2.6.22-rc7 with my neighbouring
PATCH 1/2, but they could be applied independently too.


Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>

---

diff -Nurp 2.6.22-rc7-1_2/net/ipv6/addrconf.c 2.6.22-rc7-2_2/net/ipv6/addrconf.c
--- 2.6.22-rc7-1_2/net/ipv6/addrconf.c	2007-07-05 12:33:34.000000000 +0200
+++ 2.6.22-rc7-2_2/net/ipv6/addrconf.c	2007-07-05 12:39:51.000000000 +0200
@@ -477,7 +477,7 @@ void inet6_ifa_finish_destroy(struct ine
 	in6_dev_put(ifp->idev);
 
 	if (del_timer(&ifp->timer))
-		printk("Timer is still running, when freeing ifa=%p\n", ifp);
+		printk("Timer is still pending, when freeing ifa=%p\n", ifp);
 
 	if (!ifp->dead) {
 		printk("Freeing alive inet6 address %p\n", ifp);
@@ -698,7 +698,9 @@ static void ipv6_del_addr(struct inet6_i
 
 	atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifp);
 
+	spin_lock_bh(&ifp->lock);
 	addrconf_del_timer(ifp);
+	spin_unlock_bh(&ifp->lock);
 
 	/*
 	 * Purge or update corresponding prefix
@@ -2427,7 +2429,9 @@ static int addrconf_ifdown(struct net_de
 			if (ifa->idev == idev) {
 				*bifa = ifa->lst_next;
 				ifa->lst_next = NULL;
+				spin_lock_bh(&ifa->lock);
 				addrconf_del_timer(ifa);
+				spin_unlock_bh(&ifa->lock);
 				in6_ifa_put(ifa);
 				continue;
 			}
@@ -2468,9 +2472,11 @@ static int addrconf_ifdown(struct net_de
 		idev->addr_list = ifa->if_next;
 		ifa->if_next = NULL;
 		ifa->dead = 1;
-		addrconf_del_timer(ifa);
 		write_unlock_bh(&idev->lock);
 
+		spin_lock_bh(&ifa->lock);
+		addrconf_del_timer(ifa);
+		spin_unlock_bh(&ifa->lock);
 		__ipv6_ifa_notify(RTM_DELADDR, ifa);
 		in6_ifa_put(ifa);
 
@@ -2701,8 +2707,8 @@ static void addrconf_dad_run(struct inet
 			spin_unlock_bh(&ifp->lock);
 			continue;
 		}
-		spin_unlock_bh(&ifp->lock);
 		addrconf_dad_kick(ifp);
+		spin_unlock_bh(&ifp->lock);
 	}
 	read_unlock_bh(&idev->lock);
 }

Re: [PATCH 1/2][IPV6] addrconf: fix timer deleting on exit

[David Miller]

From: Jarek Poplawski <jarkao2@o2.pl>
Date: Thu, 5 Jul 2007 13:12:46 +0200

> 
> It looks like a timer function can be running and rearm
> the timer after removing a ipv6 module.
> 
> Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>

This is completely academic as ipv6 as a module cannot
be removed, and it's been that way for years :-)

Re: [PATCH 1/2][IPV6] addrconf: fix timer deleting on exit

[Jarek Poplawski]

On Thu, Jul 05, 2007 at 02:37:40PM -0700, David Miller wrote:
> From: Jarek Poplawski <jarkao2@o2.pl>
> Date: Thu, 5 Jul 2007 13:12:46 +0200
> 
> > 
> > It looks like a timer function can be running and rearm
> > the timer after removing a ipv6 module.
> > 
> > Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>
> 
> This is completely academic as ipv6 as a module cannot
> be removed, and it's been that way for years :-)
> 

Very nice! This is what I've always dreamed about:
pure programming without any practical usability.

My other big dream is that some beautiful day linux
will have separate arch for this, e.g. pure64...

Thanks,
Jarek P.