Re: [PATCH IPv6] Fix race condition in ipv6_add_addr
From: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> Date: Tue, 29 Aug 2006 18:21:28 +0900 (JST) > In article <[EMAIL PROTECTED]> (at Tue, 29 Aug 2006 10:35:36 +0200), Olaf > Kirch <[EMAIL PROTECTED]> says: > > > During stress testing, machines were frequently crashing in > > __ipv6_ifa_notify on dst_hold(&ifp->rt.u_dst), with ifp->rt being a > > NULL pointer. > > > > The attached patch fixes the problem. > > Acked-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> Applied, thanks everyone. Although the only reason this is a problem is due to the fact that we use rwlock's here. If addrconf_lock were a normal spinlock, or taken as a writer in this code path, the problematic cases would not be possible. I guess this is a reminder that I need to revisit my patches which move all IPV6 address and inet6_dev changes out of software interrupt context, plus add use of RCU. :-) - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH IPv6] Fix race condition in ipv6_add_addr
In article <[EMAIL PROTECTED]> (at Tue, 29 Aug 2006 10:35:36 +0200), Olaf Kirch <[EMAIL PROTECTED]> says: > During stress testing, machines were frequently crashing in > __ipv6_ifa_notify on dst_hold(&ifp->rt.u_dst), with ifp->rt being a > NULL pointer. > > The attached patch fixes the problem. Acked-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> -- YOSHIFUJI Hideaki @ USAGI Project <[EMAIL PROTECTED]> GPG-FP : 9022 65EB 1ECF 3AD1 0BDF 80D8 4807 F894 E062 0EEA - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH IPv6] Fix race condition in ipv6_add_addr
Here's a patch originally from Keir Fraser, which we included in SLES10, but which we forgot to submit upstream so far. During stress testing, machines were frequently crashing in __ipv6_ifa_notify on dst_hold(&ifp->rt.u_dst), with ifp->rt being a NULL pointer. The attached patch fixes the problem. Thanks, Olaf -- Olaf Kirch | --- o --- Nous sommes du soleil we love when we play [EMAIL PROTECTED] |/ | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax From: Keir Fraser <[EMAIL PROTECTED]> Subject: ipv6_add_addr should install dstentry earlier ipv6_add_addr allocates a struct inet6_ifaddr and a dstentry, but it doesn't install the dstentry in ifa->rt until after it releases the addrconf_hash_lock. This means other CPUs will be able to see the new address while it hasn't been initialized completely yet. One possible fix would be to grab the ifp->lock spinlock when creating the address struct; a simpler fix is to just move the assignment. Acked-by: [EMAIL PROTECTED] Acked-by: [EMAIL PROTECTED] --- linux-2.6.16.13-old/net/ipv6/addrconf.c 2006-05-02 22:38:44.0 +0100 +++ linux-2.6.16.13-new/net/ipv6/addrconf.c 2006-06-18 10:16:50.0 +0100 @@ -549,6 +549,8 @@ ifa->flags = flags | IFA_F_TENTATIVE; ifa->cstamp = ifa->tstamp = jiffies; + ifa->rt = rt; + ifa->idev = idev; in6_dev_hold(idev); /* For caller */ @@ -575,8 +577,6 @@ } #endif - ifa->rt = rt; - in6_ifa_hold(ifa); write_unlock(&idev->lock); out2:
