This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] Fix double-checked locking in _res_hconf_reorder_addrs [BZ #19074]
- From: Florian Weimer <fweimer at redhat dot com>
- To: GNU C Library <libc-alpha at sourceware dot org>
- Cc: Torvald Riegel <triegel at redhat dot com>
- Date: Tue, 6 Oct 2015 14:32:07 +0200
- Subject: [PATCH] Fix double-checked locking in _res_hconf_reorder_addrs [BZ #19074]
- Authentication-results: sourceware.org; auth=none
This addresses a data race in libresolv. The race is not entirely
benign, it could cause programs to access a partially initialized
ifaddrs array.
I made sure this compiles on x86_64, but this code is difficult to test.
A potential follow-up optimization would be to move the socket creation
under the lock. No need to create a socket if we lose the race to the lock.
Florian
2015-10-06 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 3852e7f..412effe 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..11e2f2d 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
@@ -393,6 +394,7 @@ _res_hconf_reorder_addrs (struct hostent *hp)
int i, j;
/* Number of interfaces. */
static int num_ifs = -1;
+ int num_ifs_local;
/* We need to protect the dynamic buffer handling. */
__libc_lock_define_initialized (static, lock);
@@ -404,7 +406,8 @@ _res_hconf_reorder_addrs (struct hostent *hp)
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;
@@ -422,7 +425,7 @@ _res_hconf_reorder_addrs (struct hostent *hp)
__libc_lock_lock (lock);
/* Recheck, somebody else might have done the work by now. */
- if (num_ifs <= 0)
+ if (atomic_load_acquire (&num_ifs) <= 0)
{
int new_num_ifs = 0;
@@ -472,7 +475,8 @@ _res_hconf_reorder_addrs (struct hostent *hp)
/* Release lock, preserve error value, and close socket. */
errno = save;
- num_ifs = new_num_ifs;
+ atomic_store_release (&num_ifs, new_num_ifs);
+ num_ifs_local = new_num_ifs;
}
__libc_lock_unlock (lock);
@@ -480,7 +484,7 @@ _res_hconf_reorder_addrs (struct hostent *hp)
__close (sd);
}
- if (num_ifs == 0)
+ if (num_ifs_local == 0)
return;
/* Find an address for which we have a direct connection. */
@@ -488,7 +492,7 @@ _res_hconf_reorder_addrs (struct hostent *hp)
{
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;
--
2.4.3