2015-10-07  Florian Weimer  <fweimer@redhat.com>

	[BZ #19074]
	* resolv/res_hconf.c (_res_hconf_reorder_addrs): Use atomics to
	load and store num_ifs.

diff --git a/NEWS b/NEWS
index a0a91b5..805c7b9 100644
--- a/NEWS
+++ b/NEWS
@@ -49,7 +49,7 @@ Version 2.22
   18533, 18534, 18536, 18539, 18540, 18542, 18544, 18545, 18546, 18547,
   18549, 18553, 18557, 18558, 18569, 18583, 18585, 18586, 18592, 18593,
   18594, 18602, 18612, 18613, 18619, 18633, 18635, 18641, 18643, 18648,
-  18657, 18676, 18694, 18696, 18887.
+  18657, 18676, 18694, 18696, 18887, 19074.
 
 * Cache information can be queried via sysconf() function on s390 e.g. with
   _SC_LEVEL1_ICACHE_SIZE as argument.

diff --git a/resolv/res_hconf.c b/resolv/res_hconf.c
index 692d948..1497b05 100644
--- a/resolv/res_hconf.c
+++ b/resolv/res_hconf.c
@@ -45,6 +45,7 @@
 #include "ifreq.h"
 #include "res_hconf.h"
 #include <wchar.h>
+#include <atomic.h>
 
 #if IS_IN (libc)
 # define fgets_unlocked __fgets_unlocked
@@ -369,148 +370,191 @@ libc_freeres_ptr (
 static struct netaddr
 {
   int addrtype;
   union
   {
     struct
     {
       u_int32_t	addr;
       u_int32_t	mask;
     } ipv4;
   } u;
 } *ifaddrs);
 # endif
 
 /* Reorder addresses returned in a hostent such that the first address
    is an address on the local subnet, if there is such an address.
    Otherwise, nothing is changed.
 
    Note that this function currently only handles IPv4 addresses.  */
 
 void
 _res_hconf_reorder_addrs (struct hostent *hp)
 {
 #if defined SIOCGIFCONF && defined SIOCGIFNETMASK
   int i, j;
-  /* Number of interfaces.  */
+  /* Number of interfaces.  Also serves as a flag for the
+     double-checked locking idiom.  */
   static int num_ifs = -1;
-  /* We need to protect the dynamic buffer handling.  */
+  /* Local copy of num_ifs, for non-atomic access.  */
+  int num_ifs_local;
+  /* We need to protect the dynamic buffer handling.  The lock is only
+     taken during initialization, afterwards, a positive num_ifs value
+     indicates initialization.  */
   __libc_lock_define_initialized (static, lock);
 
   /* Only reorder if we're supposed to.  */
   if ((_res_hconf.flags & HCONF_FLAG_REORDER) == 0)
     return;
 
   /* Can't deal with anything but IPv4 for now...  */
   if (hp->h_addrtype != AF_INET)
     return;
 
-  if (num_ifs <= 0)
+  num_ifs_local = atomic_load_acquire (&num_ifs);
+  if (num_ifs_local <= 0)
     {
       struct ifreq *ifr, *cur_ifr;
       int sd, num, i;
       /* Save errno.  */
       int save = errno;
 
       /* Initialize interface table.  */
 
       /* The SIOCGIFNETMASK ioctl will only work on an AF_INET socket.  */
       sd = __socket (AF_INET, SOCK_DGRAM, 0);
       if (sd < 0)
 	return;
 
       /* Get lock.  */
       __libc_lock_lock (lock);
 
-      /* Recheck, somebody else might have done the work by now.  */
-      if (num_ifs <= 0)
+      /* Recheck, somebody else might have done the work by now.  A
+	 regular load is sufficient because we have the lock, and
+	 num_ifs is only updated under the lock.  */
+      num_ifs_local = num_ifs;
+      if (num_ifs_local <= 0)
 	{
+	  /* This is the only block which writes to num_ifs.  It can
+	     be executed several times (sequentially) if
+	     initialization does not yield any interfaces, and num_ifs
+	     remains zero.  However, once we stored a positive value
+	     in num_ifs below, this block cannot be entered again due
+	     to the condition above.  */
 	  int new_num_ifs = 0;
 
 	  /* Get a list of interfaces.  */
 	  __ifreq (&ifr, &num, sd);
 	  if (!ifr)
 	    goto cleanup;
 
 	  ifaddrs = malloc (num * sizeof (ifaddrs[0]));
 	  if (!ifaddrs)
 	    goto cleanup1;
 
 	  /* Copy usable interfaces in ifaddrs structure.  */
 	  for (cur_ifr = ifr, i = 0; i < num;
 	       cur_ifr = __if_nextreq (cur_ifr), ++i)
 	    {
 	      union
 	      {
 		struct sockaddr sa;
 		struct sockaddr_in sin;
 	      } ss;
 
 	      if (cur_ifr->ifr_addr.sa_family != AF_INET)
 		continue;
 
 	      ifaddrs[new_num_ifs].addrtype = AF_INET;
 	      ss.sa = cur_ifr->ifr_addr;
 	      ifaddrs[new_num_ifs].u.ipv4.addr = ss.sin.sin_addr.s_addr;
 
 	      if (__ioctl (sd, SIOCGIFNETMASK, cur_ifr) < 0)
 		continue;
 
 	      ss.sa = cur_ifr->ifr_netmask;
 	      ifaddrs[new_num_ifs].u.ipv4.mask = ss.sin.sin_addr.s_addr;
 
 	      /* Now we're committed to this entry.  */
 	      ++new_num_ifs;
 	    }
 	  /* Just keep enough memory to hold all the interfaces we want.  */
 	  ifaddrs = realloc (ifaddrs, new_num_ifs * sizeof (ifaddrs[0]));
 	  assert (ifaddrs != NULL);
 
 	cleanup1:
 	  __if_freereq (ifr, num);
 
 	cleanup:
 	  /* Release lock, preserve error value, and close socket.  */
 	  errno = save;
 
-	  num_ifs = new_num_ifs;
+	  /* Advertise successful initialization if new_num_ifs is
+	     positive (and no updates to ifaddrs are permitted after
+	     that).  Otherwise, num_ifs remains unchanged, at
+	     zero.  */
+	  atomic_store_release (&num_ifs, new_num_ifs);
+	  /* Keep the local copy current, to save another load.  */
+	  num_ifs_local = new_num_ifs;
 	}
 
       __libc_lock_unlock (lock);
 
       __close (sd);
     }
 
-  if (num_ifs == 0)
+  /* num_ifs_local cannot be negative because the if statement above
+     covered this case.  It can still be zero if we just performed
+     initialization, but could not find any interfaces.  */
+  if (num_ifs_local == 0)
     return;
 
+  /* At this point, access to ifaddrs is free from data races:
+
+     (1) num_ifs is positive, so we know the initialization code has
+         completed in some thread because the release-store to num_ifs
+         is dependency-ordered before the acquire-load.  (If this
+         thread performed the initialization, it is sequence-ordered
+         before this point.)
+
+     (2) The initialization of ifaddrs (both of ifaddrs itself and the
+         data pointed to it) is sequenced before the write to the
+         update to new_num_ifs.
+
+     (3) Any potential write would have to happen in the
+         initialization code, but no consistent execution can re-enter
+         it after num_ifs is positive.
+
+     Consequently, all potential writes to ifaddrs (and the data it
+     points to) happened before this point.  */
+
   /* Find an address for which we have a direct connection.  */
   for (i = 0; hp->h_addr_list[i]; ++i)
     {
       struct in_addr *haddr = (struct in_addr *) hp->h_addr_list[i];
 
-      for (j = 0; j < num_ifs; ++j)
+      for (j = 0; j < num_ifs_local; ++j)
 	{
 	  u_int32_t if_addr    = ifaddrs[j].u.ipv4.addr;
 	  u_int32_t if_netmask = ifaddrs[j].u.ipv4.mask;
 
 	  if (((haddr->s_addr ^ if_addr) & if_netmask) == 0)
 	    {
 	      void *tmp;
 
 	      tmp = hp->h_addr_list[i];
 	      hp->h_addr_list[i] = hp->h_addr_list[0];
 	      hp->h_addr_list[0] = tmp;
 	      return;
 	    }
 	}
     }
 #endif /* defined(SIOCGIFCONF) && ... */
 }
 
 
 /* If HOSTNAME has a postfix matching any of the trimdomains, trim away
    that postfix.  Notice that HOSTNAME is modified inplace.  Also, the
    original code applied all trimdomains in order, meaning that the
    same domainname could be trimmed multiple times.  I believe this
    was unintentional.  */
 void
