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>

1 files changed, 38 insertions, 8 deletions

diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
index 9c6e1af16..89ff6b46e 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;
 }