ipv6: prevent race between address creation and removal
authorJiri Benc <jbenc@redhat.com>
Thu, 1 Aug 2013 08:41:28 +0000 (10:41 +0200)
committerDavid S. Miller <davem@davemloft.net>
Thu, 1 Aug 2013 21:16:20 +0000 (14:16 -0700)
There's a race in IPv6 automatic addess assignment. The address is created
with zero lifetime when it's added to various address lists. Before it gets
assigned the correct lifetime, there's a window where a new address may be
configured. This causes the semi-initiated address to be deleted in
addrconf_verify.

This was discovered as a reference leak caused by concurrent run of
__ipv6_ifa_notify for both RTM_NEWADDR and RTM_DELADDR with the same
address.

Fix this by setting the lifetime before the address is added to
inet6_addr_lst.

A few notes:

1. In addrconf_prefix_rcv, by setting update_lft to zero, the
   if (update_lft) { ... } condition is no longer executed for newly
   created addresses. This is okay, as the ifp fields are set in
   ipv6_add_addr now and ipv6_ifa_notify is called (and has been called)
   through addrconf_dad_start.

2. The removal of the whole block under ifp->lock in inet6_addr_add is okay,
   too, as tstamp is initialized to jiffies in ipv6_add_addr.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/ipv6/addrconf.c

index a0ce957fb671cc80b6baac6b551a511e52e19336..da4241c8c7dafe0004a53ed85dfc270cd3be16ba 100644 (file)
@@ -815,7 +815,7 @@ static u32 inet6_addr_hash(const struct in6_addr *addr)
 static struct inet6_ifaddr *
 ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
              const struct in6_addr *peer_addr, int pfxlen,
-             int scope, u32 flags)
+             int scope, u32 flags, u32 valid_lft, u32 prefered_lft)
 {
        struct inet6_ifaddr *ifa = NULL;
        struct rt6_info *rt;
@@ -875,6 +875,8 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
        ifa->scope = scope;
        ifa->prefix_len = pfxlen;
        ifa->flags = flags | IFA_F_TENTATIVE;
+       ifa->valid_lft = valid_lft;
+       ifa->prefered_lft = prefered_lft;
        ifa->cstamp = ifa->tstamp = jiffies;
        ifa->tokenized = false;
 
@@ -1127,7 +1129,8 @@ retry:
        ift = !max_addresses ||
              ipv6_count_addresses(idev) < max_addresses ?
                ipv6_add_addr(idev, &addr, NULL, tmp_plen,
-                             ipv6_addr_scope(&addr), addr_flags) : NULL;
+                             ipv6_addr_scope(&addr), addr_flags,
+                             tmp_valid_lft, tmp_prefered_lft) : NULL;
        if (IS_ERR_OR_NULL(ift)) {
                in6_ifa_put(ifp);
                in6_dev_put(idev);
@@ -1139,8 +1142,6 @@ retry:
 
        spin_lock_bh(&ift->lock);
        ift->ifpub = ifp;
-       ift->valid_lft = tmp_valid_lft;
-       ift->prefered_lft = tmp_prefered_lft;
        ift->cstamp = now;
        ift->tstamp = tmp_tstamp;
        spin_unlock_bh(&ift->lock);
@@ -2185,14 +2186,16 @@ ok:
                                ifp = ipv6_add_addr(in6_dev, &addr, NULL,
                                                    pinfo->prefix_len,
                                                    addr_type&IPV6_ADDR_SCOPE_MASK,
-                                                   addr_flags);
+                                                   addr_flags, valid_lft,
+                                                   prefered_lft);
 
                        if (IS_ERR_OR_NULL(ifp)) {
                                in6_dev_put(in6_dev);
                                return;
                        }
 
-                       update_lft = create = 1;
+                       update_lft = 0;
+                       create = 1;
                        ifp->cstamp = jiffies;
                        ifp->tokenized = tokenized;
                        addrconf_dad_start(ifp);
@@ -2213,7 +2216,7 @@ ok:
                                stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
                        else
                                stored_lft = 0;
-                       if (!update_lft && stored_lft) {
+                       if (!update_lft && !create && stored_lft) {
                                if (valid_lft > MIN_VALID_LIFETIME ||
                                    valid_lft > stored_lft)
                                        update_lft = 1;
@@ -2459,15 +2462,10 @@ static int inet6_addr_add(struct net *net, int ifindex, const struct in6_addr *p
                prefered_lft = timeout;
        }
 
-       ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags);
+       ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags,
+                           valid_lft, prefered_lft);
 
        if (!IS_ERR(ifp)) {
-               spin_lock_bh(&ifp->lock);
-               ifp->valid_lft = valid_lft;
-               ifp->prefered_lft = prefered_lft;
-               ifp->tstamp = jiffies;
-               spin_unlock_bh(&ifp->lock);
-
                addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
                                      expires, flags);
                /*
@@ -2559,7 +2557,8 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
 {
        struct inet6_ifaddr *ifp;
 
-       ifp = ipv6_add_addr(idev, addr, NULL, plen, scope, IFA_F_PERMANENT);
+       ifp = ipv6_add_addr(idev, addr, NULL, plen,
+                           scope, IFA_F_PERMANENT, 0, 0);
        if (!IS_ERR(ifp)) {
                spin_lock_bh(&ifp->lock);
                ifp->flags &= ~IFA_F_TENTATIVE;
@@ -2685,7 +2684,7 @@ static void addrconf_add_linklocal(struct inet6_dev *idev, const struct in6_addr
 #endif
 
 
-       ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags);
+       ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags, 0, 0);
        if (!IS_ERR(ifp)) {
                addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
                addrconf_dad_start(ifp);