Avoid race in nscd

2012-05-15  Jeff Law  <law@redhat.com>
            Andreas Jaeger  <aj@suse.de>

        [BZ #13594]
        * nscd/nscd-client.h (__nscd_acquire_maplock): New function, split
        out from...
        * nscd/nscd_helper.c (__nscd_get_map_ref): ... here.
        * nscd/nscd-client.h: Add __nscd_acquire_maplock.
        * nscd/nscd_gethst_r.c (__nscd_get_nl_timestamp): Add locking to
        code changing __hst_map_handle.map.

ChangeLog

@@ -1,3 +1,14 @@
+2012-05-15  Jeff Law  <law@redhat.com>
+	    Andreas Jaeger  <aj@suse.de>
+
+	[BZ #13594]
+	* nscd/nscd-client.h (__nscd_acquire_maplock): New function, split
+	out from...
+	* nscd/nscd_helper.c (__nscd_get_map_ref): ... here.
+	* nscd/nscd-client.h: Add __nscd_acquire_maplock.
+	* nscd/nscd_gethst_r.c (__nscd_get_nl_timestamp): Add locking to
+	code changing __hst_map_handle.map.
+
 2012-05-15  Roland McGrath  <roland@hack.frob.com>
 
 	* configure.in (sysnames): Look for Implies-before and Implies-after

NEWS

@@ -18,16 +18,16 @@ Version 2.16
   11494, 11521, 11677, 11837, 11959, 12047, 12097, 12193, 12297, 12298,
   12301, 12340, 12354, 13058, 13361, 13525, 13526, 13527, 13528, 13529,
   13530, 13531, 13532, 13533, 13547, 13551, 13552, 13553, 13555, 13556,
-  13559, 13563, 13566, 13583, 13592, 13613, 13618, 13637, 13656, 13658,
+  13559, 13563, 13566, 13583, 13592, 13594, 13613, 13618, 13637, 13656,
-  13673, 13691, 13695, 13704, 13705, 13706, 13726, 13738, 13739, 13750,
+  13658, 13673, 13691, 13695, 13704, 13705, 13706, 13726, 13738, 13739,
-  13758, 13760, 13761, 13775, 13786, 13787, 13792, 13806, 13824, 13840,
+  13750, 13758, 13760, 13761, 13775, 13786, 13787, 13792, 13806, 13824,
-  13841, 13844, 13846, 13851, 13852, 13854, 13871, 13872, 13873, 13879,
+  13840, 13841, 13844, 13846, 13851, 13852, 13854, 13871, 13872, 13873,
-  13883, 13884, 13885, 13886, 13892, 13895, 13908, 13910, 13911, 13912,
+  13879, 13883, 13884, 13885, 13886, 13892, 13895, 13908, 13910, 13911,
-  13913, 13914, 13915, 13916, 13917, 13918, 13919, 13920, 13921, 13922,
+  13912, 13913, 13914, 13915, 13916, 13917, 13918, 13919, 13920, 13921,
-  13923, 13924, 13926, 13927, 13928, 13938, 13941, 13942, 13954, 13955,
+  13922, 13923, 13924, 13926, 13927, 13928, 13938, 13941, 13942, 13954,
-  13956, 13963, 13967, 13970, 13973, 13979, 13983, 13986, 14012, 14027,
+  13955, 13956, 13963, 13967, 13970, 13973, 13979, 13983, 13986, 14012,
-  14033, 14034, 14040, 14043, 14044, 14049, 14053, 14055, 14064, 14080,
+  14027, 14033, 14034, 14040, 14043, 14044, 14049, 14053, 14055, 14064,
-  14083, 14103, 14104
+  14080, 14083, 14103, 14104
 
 * ISO C11 support:
 

nscd/nscd-client.h

@@ -1,5 +1,4 @@
-/* Copyright (c) 1998, 1999, 2000, 2003, 2004, 2005, 2006, 2007, 2009, 2011
+/* Copyright (c) 1998-2012 Free Software Foundation, Inc.
-   Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Thorsten Kukuk <kukuk@suse.de>, 1998.
 
@@ -322,6 +321,24 @@ struct locked_map_ptr
 };
 #define libc_locked_map_ptr(class, name) class struct locked_map_ptr name
 
+/* Try acquiring lock for mapptr, returns true if it succeeds, false
+   if not.  */
+static inline bool __nscd_acquire_maplock (volatile struct locked_map_ptr *mapptr)
+{
+  int cnt = 0;
+  while (__builtin_expect (atomic_compare_and_exchange_val_acq (&mapptr->lock,
+								1, 0) != 0, 0))
+    {
+      // XXX Best number of rounds?
+      if (__builtin_expect (++cnt > 5, 0))
+	return false;
+
+      atomic_delay ();
+    }
+
+  return true;
+}
+
 
 /* Open socket connection to nscd server.  */
 extern int __nscd_open_socket (const char *key, size_t keylen,

nscd/nscd_gethst_r.c

@@ -1,5 +1,4 @@
-/* Copyright (C) 1998-2005, 2006, 2007, 2008, 2009, 2011
+/* Copyright (C) 1998-2012 Free Software Foundation, Inc.
-   Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@cygnus.com>, 1998.
 
@@ -100,9 +99,18 @@ libc_freeres_fn (hst_map_free)
 uint32_t
 __nscd_get_nl_timestamp (void)
 {
+  uint32_t retval;
   if (__nss_not_use_nscd_hosts != 0)
     return 0;
 
+  /* __nscd_get_mapping can change hst_map_handle.mapped to NO_MAPPING.
+   However, __nscd_get_mapping assumes the prior value was not NO_MAPPING.
+   Thus we have to acquire the lock to prevent this thread from changing
+   hst_map_handle.mapped to NO_MAPPING while another thread is inside
+    __nscd_get_mapping.  */
+  if (!__nscd_acquire_maplock (&__hst_map_handle))
+    return 0;
+
   struct mapped_database *map = __hst_map_handle.mapped;
 
   if (map == NULL
@@ -112,9 +120,14 @@ __nscd_get_nl_timestamp (void)
     map = __nscd_get_mapping (GETFDHST, "hosts", &__hst_map_handle.mapped);
 
   if (map == NO_MAPPING)
-    return 0;
+    retval = 0;
+  else
+    retval = map->head->extra_data[NSCD_HST_IDX_CONF_TIMESTAMP];
 
-  return map->head->extra_data[NSCD_HST_IDX_CONF_TIMESTAMP];
+  /* Release the lock.  */
+  __hst_map_handle.lock = 0;
+
+  return retval;
 }
 
 

nscd/nscd_helper.c

@@ -1,4 +1,4 @@
-/* Copyright (C) 1998-2007, 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 1998-2012 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@cygnus.com>, 1998.
 
@@ -419,7 +419,6 @@ __nscd_get_mapping (request_type type, const char *key,
   return result;
 }
 
-
 struct mapped_database *
 __nscd_get_map_ref (request_type type, const char *name,
 		    volatile struct locked_map_ptr *mapptr, int *gc_cyclep)
@@ -428,16 +427,8 @@ __nscd_get_map_ref (request_type type, const char *name,
   if (cur == NO_MAPPING)
     return cur;
 
-  int cnt = 0;
+  if (!__nscd_acquire_maplock (mapptr))
-  while (__builtin_expect (atomic_compare_and_exchange_val_acq (&mapptr->lock,
+    return NO_MAPPING;
-								1, 0) != 0, 0))
-    {
-      // XXX Best number of rounds?
-      if (__builtin_expect (++cnt > 5, 0))
-	return NO_MAPPING;
-
-      atomic_delay ();
-    }
 
   cur = mapptr->mapped;