This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] #13594: Avoid race in nscd
On 05/14/2012 10:32 PM, Roland McGrath wrote:
>> This is the same code as used in nscd/nscd_helper.c.
>
> Ok. It would be better to write a shared subroutine for it.
> But that doesn't have to hold up this change.
>
>> + retval = map->head->extra_data[NSCD_HST_IDX_CONF_TIMESTAMP];
>
> Excess space here.
fixed.
> Fine to commit with that one nit fix. If you want to break out the locking
> loop into a subroutine first, I'd like that even better.
Here's a patch for the breakout, could you review this, please?
thanks,
Andreas
2012-05-14 Jeff Law <law@redhat.com>
Andreas Jaeger <aj@suse.de>
[BZ #13594]
* nscd/nscd_helper.c (__nscd_acquire_maplock): New function, split
out from...
(__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.
diff --git a/nscd/nscd-client.h b/nscd/nscd-client.h
index e57a23c..7c638b2 100644
--- a/nscd/nscd-client.h
+++ b/nscd/nscd-client.h
@@ -1,5 +1,4 @@
-/* Copyright (c) 1998, 1999, 2000, 2003, 2004, 2005, 2006, 2007, 2009, 2011
- Free Software Foundation, Inc.
+/* Copyright (c) 1998-2012 Free Software Foundation, Inc.
This file is part of the GNU C Library.
Contributed by Thorsten Kukuk <kukuk@suse.de>, 1998.
@@ -322,6 +321,9 @@ struct locked_map_ptr
};
#define libc_locked_map_ptr(class, name) class struct locked_map_ptr name
+/* Try acquiring lock for mapptr, return 1 if not possible. */
+extern int __nscd_acquire_maplock (struct locked_map_ptr *mapptr) attribute_hidden;
+
/* Open socket connection to nscd server. */
extern int __nscd_open_socket (const char *key, size_t keylen,
diff --git a/nscd/nscd_gethst_r.c b/nscd/nscd_gethst_r.c
index c1661f8..c358286 100644
--- a/nscd/nscd_gethst_r.c
+++ b/nscd/nscd_gethst_r.c
@@ -1,5 +1,4 @@
-/* Copyright (C) 1998-2005, 2006, 2007, 2008, 2009, 2011
- 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.
@@ -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];
+
+ /* Release the lock. */
+ __hst_map_handle.lock = 0;
- return map->head->extra_data[NSCD_HST_IDX_CONF_TIMESTAMP];
+ return retval;
}
diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
index 92558b6..2178524 100644
--- a/nscd/nscd_helper.c
+++ b/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,26 +419,34 @@ __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)
+int
+__nscd_acquire_maplock (struct locked_map_ptr *mapptr)
{
- struct mapped_database *cur = mapptr->mapped;
- if (cur == NO_MAPPING)
- return cur;
-
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 NO_MAPPING;
+ return 1;
atomic_delay ();
}
+ return 0;
+}
+
+struct mapped_database *
+__nscd_get_map_ref (request_type type, const char *name,
+ volatile struct locked_map_ptr *mapptr, int *gc_cyclep)
+{
+ struct mapped_database *cur = mapptr->mapped;
+ if (cur == NO_MAPPING)
+ return cur;
+
+ if (__nscd_acquire_maplock (&mapptr))
+ return NO_MAPPING;
+
cur = mapptr->mapped;
if (__builtin_expect (cur != NO_MAPPING, 1))
--
Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
GPG fingerprint = 93A3 365E CE47 B889 DF7F FED1 389A 563C C272 A126