summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLukas Slebodnik <lslebodn@redhat.com>2014-11-21 11:28:36 +0100
committerJakub Hrozek <jhrozek@redhat.com>2014-11-24 20:54:00 +0100
commit6a60e29468fc6b4043a4dc52d3aab73e8465db70 (patch)
tree8c79f852113b017fe6a875ff2259b67fb38b71ee
parent19f6a6733b5c6cf7dd2f6f746cfa5c787706331c (diff)
downloadsssd-6a60e29468fc6b4043a4dc52d3aab73e8465db70.zip
sssd-6a60e29468fc6b4043a4dc52d3aab73e8465db70.tar.gz
sssd-6a60e29468fc6b4043a4dc52d3aab73e8465db70.tar.xz
sss_client: Fix race condition in memory cache
Thread safe initialisation was fixed in ticket #2380, but there is still race condition in reinitialisation. If caches is invalidated with command sss_cache -U (-G or -E) then client code will need to reinitialize fast memory cache. Let say we have two threads. The 1st thread find out that memory cache should be reinitialized; therefore the fast memory cached is unmapped and context destroyed. In the same time, 2nd thread tried to check header of memory cache whether it is initialized and valid. As a result of previously unmapped memory the 2nd thread access out of bound memory (SEGFAULT). The destroying of fast memory cache cannot be done any time. We need to be sure that there isn't any other thread which uses mmaped memory. The new counter of active threads was added for this purpose. The state of fast memory cache was converted from boolean to three value state (UNINITIALIZED, INITIALIZED, RECYCLED) UNINITIALIZED - the fast memory cache need to be initialized. - if there is a problem with initialisation the state will not change - after successful initialisation, the state will change to INITIALIZED INITIALIZED - if the cahe was invalidated or there is any other problem was detected in memory cache header the state will change to RECYCLED and memory cache IS NOT destroyed. RECYCLED - nothing will be done is there are any active threads which may use the data from mmaped memory - if there aren't active threads the fast memory cahe is destroyed and state is changed to UNINITIALIZED. https://fedorahosted.org/sssd/ticket/2445 Reviewed-by: Michal Židek <mzidek@redhat.com>
-rw-r--r--src/sss_client/nss_mc.h10
-rw-r--r--src/sss_client/nss_mc_common.c46
-rw-r--r--src/sss_client/nss_mc_group.c8
-rw-r--r--src/sss_client/nss_mc_passwd.c8
4 files changed, 59 insertions, 13 deletions
diff --git a/src/sss_client/nss_mc.h b/src/sss_client/nss_mc.h
index 685cc41..050bd41 100644
--- a/src/sss_client/nss_mc.h
+++ b/src/sss_client/nss_mc.h
@@ -33,9 +33,15 @@
typedef int errno_t;
#endif
+enum sss_mc_state {
+ UNINITIALIZED = 0,
+ INITIALIZED,
+ RECYCLED,
+};
+
/* common stuff */
struct sss_cli_mc_ctx {
- bool initialized;
+ enum sss_mc_state initialized;
int fd;
uint32_t seed; /* seed from the tables header */
@@ -48,6 +54,8 @@ struct sss_cli_mc_ctx {
uint32_t *hash_table; /* hash table address (in mmap) */
uint32_t ht_size; /* size of hash table */
+
+ uint32_t active_threads; /* count of threads which use memory cache */
};
errno_t sss_nss_mc_get_ctx(const char *name, struct sss_cli_mc_ctx *ctx);
diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
index 9c6e1af..89ff6b4 100644
--- a/src/sss_client/nss_mc_common.c
+++ b/src/sss_client/nss_mc_common.c
@@ -123,7 +123,7 @@ static errno_t sss_nss_mc_init_ctx(const char *name,
sss_nss_lock();
/* check if ctx is initialised by previous thread. */
- if (ctx->initialized) {
+ if (ctx->initialized != UNINITIALIZED) {
ret = sss_nss_check_header(ctx);
goto done;
}
@@ -163,7 +163,7 @@ static errno_t sss_nss_mc_init_ctx(const char *name,
goto done;
}
- ctx->initialized = true;
+ ctx->initialized = INITIALIZED;
ret = 0;
@@ -181,22 +181,52 @@ errno_t sss_nss_mc_get_ctx(const char *name, struct sss_cli_mc_ctx *ctx)
{
char *envval;
int ret;
+ bool need_decrement = false;
envval = getenv("SSS_NSS_USE_MEMCACHE");
if (envval && strcasecmp(envval, "NO") == 0) {
return EPERM;
}
- if (ctx->initialized) {
+ switch (ctx->initialized) {
+ case UNINITIALIZED:
+ __sync_add_and_fetch(&ctx->active_threads, 1);
+ ret = sss_nss_mc_init_ctx(name, ctx);
+ if (ret) {
+ need_decrement = true;
+ }
+ break;
+ case INITIALIZED:
+ __sync_add_and_fetch(&ctx->active_threads, 1);
ret = sss_nss_check_header(ctx);
- goto done;
+ if (ret) {
+ need_decrement = true;
+ }
+ break;
+ case RECYCLED:
+ /* we need to safely destroy memory cache */
+ ret = EAGAIN;
+ break;
+ default:
+ ret = EFAULT;
}
- ret = sss_nss_mc_init_ctx(name, ctx);
-
-done:
if (ret) {
- sss_nss_mc_destroy_ctx(ctx);
+ if (ctx->initialized == INITIALIZED) {
+ ctx->initialized = RECYCLED;
+ }
+ if (ctx->initialized == RECYCLED && ctx->active_threads == 0) {
+ /* just one thread should call munmap */
+ sss_nss_lock();
+ if (ctx->initialized == RECYCLED) {
+ sss_nss_mc_destroy_ctx(ctx);
+ }
+ sss_nss_unlock();
+ }
+ if (need_decrement) {
+ /* In case of error, we will not touch mmapped area => decrement */
+ __sync_sub_and_fetch(&ctx->active_threads, 1);
+ }
}
return ret;
}
diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index 268b40e..e0fdb97 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -29,7 +29,8 @@
#include "nss_mc.h"
#include "util/util_safealign.h"
-struct sss_cli_mc_ctx gr_mc_ctx = { false, -1, 0, NULL, 0, NULL, 0, NULL, 0 };
+struct sss_cli_mc_ctx gr_mc_ctx = { UNINITIALIZED, -1, 0, NULL, 0, NULL, 0,
+ NULL, 0, 0 };
static errno_t sss_nss_mc_parse_result(struct sss_mc_rec *rec,
struct group *result,
@@ -176,6 +177,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
done:
free(rec);
+ __sync_sub_and_fetch(&gr_mc_ctx.active_threads, 1);
return ret;
}
@@ -198,7 +200,8 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
len = snprintf(gidstr, 11, "%ld", (long)gid);
if (len > 10) {
- return EINVAL;
+ ret = EINVAL;
+ goto done;
}
/* hashes are calculated including the NULL terminator */
@@ -242,6 +245,7 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
done:
free(rec);
+ __sync_sub_and_fetch(&gr_mc_ctx.active_threads, 1);
return ret;
}
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index fa19afc..10e43e2 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -28,7 +28,8 @@
#include <time.h>
#include "nss_mc.h"
-struct sss_cli_mc_ctx pw_mc_ctx = { false, -1, 0, NULL, 0, NULL, 0, NULL, 0 };
+struct sss_cli_mc_ctx pw_mc_ctx = { UNINITIALIZED, -1, 0, NULL, 0, NULL, 0,
+ NULL, 0, 0 };
static errno_t sss_nss_mc_parse_result(struct sss_mc_rec *rec,
struct passwd *result,
@@ -170,6 +171,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
done:
free(rec);
+ __sync_sub_and_fetch(&pw_mc_ctx.active_threads, 1);
return ret;
}
@@ -192,7 +194,8 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
len = snprintf(uidstr, 11, "%ld", (long)uid);
if (len > 10) {
- return EINVAL;
+ ret = EINVAL;
+ goto done;
}
/* hashes are calculated including the NULL terminator */
@@ -236,6 +239,7 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
done:
free(rec);
+ __sync_sub_and_fetch(&pw_mc_ctx.active_threads, 1);
return ret;
}