[SSSD] [PATCHES] sssd_client: Do not use removed memory cache

Lukas Slebodnik lslebodn at redhat.com
Fri Nov 6 11:52:57 UTC 2015


ehlo,

attached patches fix issue with memory cache
in integration tests.
https://fedorahosted.org/sssd/ticket/2726

LS
-------------- next part --------------
>From b456f28592cb98a5043a852c707d2c5460ab2cae Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Fri, 6 Nov 2015 08:48:05 +0100
Subject: [PATCH 1/4] sss_client: Fix underflow of active_threads

If the memory cache was not initialized and there was a failure in
initialisation of memory cache context (e.g. memory cache file
does not exist) then mc_context had to be destroyed to release
resources.

However the count of active threads in sss_cli_mc_ctx is already higher
than zero because current thread is working wih the mc_context.
But this counter was zero-ed with memset in sss_nss_mc_destroy_ctx
due to issue with initialisation of memory cache.
Then we have to decrease counter of active thread in function
sss_nss_mc_get_ctx because initialisation of mc failed.
And the result of this decrement is underflow of counter.

Related to:
https://fedorahosted.org/sssd/ticket/2726
---
 src/sss_client/nss_mc_common.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
index 707d1249fe14812897a69ecf8d12a141d2fb5ed0..92f802db72112d2082d5819034d77750b3435a65 100644
--- a/src/sss_client/nss_mc_common.c
+++ b/src/sss_client/nss_mc_common.c
@@ -104,6 +104,8 @@ errno_t sss_nss_check_header(struct sss_cli_mc_ctx *ctx)
 
 static void sss_nss_mc_destroy_ctx(struct sss_cli_mc_ctx *ctx)
 {
+    uint32_t active_threads = ctx->active_threads;
+
     if ((ctx->mmap_base != NULL) && (ctx->mmap_size != 0)) {
         munmap(ctx->mmap_base, ctx->mmap_size);
     }
@@ -112,6 +114,9 @@ static void sss_nss_mc_destroy_ctx(struct sss_cli_mc_ctx *ctx)
     }
     memset(ctx, 0, sizeof(struct sss_cli_mc_ctx));
     ctx->fd = -1;
+
+    /* restore count of active threads */
+    ctx->active_threads = active_threads;
 }
 
 static errno_t sss_nss_mc_init_ctx(const char *name,
-- 
2.5.0

-------------- next part --------------
>From bce75e43bce918f107d08c338a3bd8e8ce1d7674 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Fri, 6 Nov 2015 09:39:05 +0100
Subject: [PATCH 2/4] sssd_client: Do not use removed memory cache

Resolves:
https://fedorahosted.org/sssd/ticket/2726
---
 src/sss_client/nss_mc_common.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
index 92f802db72112d2082d5819034d77750b3435a65..6cff2e1b9f640ee81850be86f458597bdb3a6bfb 100644
--- a/src/sss_client/nss_mc_common.c
+++ b/src/sss_client/nss_mc_common.c
@@ -60,6 +60,8 @@ errno_t sss_nss_check_header(struct sss_cli_mc_ctx *ctx)
     struct sss_mc_header h;
     bool copy_ok;
     int count;
+    int ret;
+    struct stat fdstat;
 
     /* retry barrier protected reading max 5 times then give up */
     for (count = 5; count > 0; count--) {
@@ -99,6 +101,16 @@ errno_t sss_nss_check_header(struct sss_cli_mc_ctx *ctx)
         }
     }
 
+    ret = fstat(ctx->fd, &fdstat);
+    if (ret == -1) {
+        return EIO;
+    }
+
+    if (fdstat.st_nlink == 0) {
+        /* memory cache was removed; we need to reinitialize it. */
+        return EINVAL;
+    }
+
     return 0;
 }
 
-- 
2.5.0

-------------- next part --------------
>From 2b5f31feee6aac24af5d86abab3540466d359438 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Fri, 6 Nov 2015 11:37:54 +0100
Subject: [PATCH 3/4] test_memory_cache: Test removing mc without invalidation

Long living clients should be able to reinitialize
memory cache which was removed but it not initialized.

This patch also remove workaround in test_local_domain.py

Test for:
https://fedorahosted.org/sssd/ticket/2726
---
 src/tests/intg/test_local_domain.py |  6 ++----
 src/tests/intg/test_memory_cache.py | 39 +++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/src/tests/intg/test_local_domain.py b/src/tests/intg/test_local_domain.py
index c62de16ce04b640503250c926d6eb3d199ed0728..4de0f5da2da38b71fef07d0a993fc4beaca39d9a 100644
--- a/src/tests/intg/test_local_domain.py
+++ b/src/tests/intg/test_local_domain.py
@@ -61,10 +61,8 @@ def create_sssd_fixture(request):
         subprocess.call(["sss_cache", "-E"])
         for path in os.listdir(config.DB_PATH):
             os.unlink(config.DB_PATH + "/" + path)
-        # FIXME: Uncomment this when ticket #2726 is solved
-        # https://fedorahosted.org/sssd/ticket/2726
-        # for path in os.listdir(config.MCACHE_PATH):
-        #     os.unlink(config.MCACHE_PATH + "/" + path)
+        for path in os.listdir(config.MCACHE_PATH):
+            os.unlink(config.MCACHE_PATH + "/" + path)
     request.addfinalizer(teardown)
 
 
diff --git a/src/tests/intg/test_memory_cache.py b/src/tests/intg/test_memory_cache.py
index a8d6d8f5c87bdea517a00b5a703b9658ebb1fe0a..d324f29fb1ddcf05a31a7eb22668234ece8e7301 100644
--- a/src/tests/intg/test_memory_cache.py
+++ b/src/tests/intg/test_memory_cache.py
@@ -743,3 +743,42 @@ def test_invalidate_everything_after_stop(ldap_conn, sanity_rfc2307):
     subprocess.call(["sss_cache", "-E"])
 
     assert_missing_mc_records_for_user1()
+
+
+def test_removed_mc(ldap_conn, sanity_rfc2307):
+    """
+    Regression test for ticket:
+    https://fedorahosted.org/sssd/ticket/2726
+    """
+
+    import grp
+    import pwd
+
+    ent.assert_passwd_by_name(
+        'user1',
+        dict(name='user1', passwd='*', uid=1001, gid=2001,
+             gecos='1001', shell='/bin/bash'))
+    ent.assert_passwd_by_uid(
+        1001,
+        dict(name='user1', passwd='*', uid=1001, gid=2001,
+             gecos='1001', shell='/bin/bash'))
+
+    ent.assert_group_by_name("group1", dict(name="group1", gid=2001))
+    ent.assert_group_by_gid(2001, dict(name="group1", gid=2001))
+    stop_sssd()
+
+    # remove cache without invalidation
+    for path in os.listdir(config.MCACHE_PATH):
+        os.unlink(config.MCACHE_PATH + "/" + path)
+
+    # sssd is stopped; so the memory cache should not be used
+    # in long living clients (py.test in this case)
+    with pytest.raises(KeyError):
+        pwd.getpwnam('user1')
+    with pytest.raises(KeyError):
+        pwd.getpwuid(1001)
+
+    with pytest.raises(KeyError):
+        grp.getgrnam('group1')
+    with pytest.raises(KeyError):
+        grp.getgrgid(2001)
-- 
2.5.0

-------------- next part --------------
>From 1dfd65bf2642b36a838760a9ece5e14b0aec04f4 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Fri, 6 Nov 2015 12:47:24 +0100
Subject: [PATCH 4/4] Revert "intg: Invalidate memory cache before removing
 files"

It is not necessary to invalidate memory cache before removing
them. The sssd_client can handle it without any problem.

This reverts commit eabc1732ef91548616a699b7e9f8d30e5e7b8dd3.
---
 src/tests/intg/ldap_test.py         | 1 -
 src/tests/intg/test_local_domain.py | 1 -
 src/tests/intg/test_memory_cache.py | 1 -
 3 files changed, 3 deletions(-)

diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py
index 6d61726b85a155a994fc6338ea3c806c54963fe1..9f1b7e0b2cea35f0419fd41015e14bb8d014db97 100644
--- a/src/tests/intg/ldap_test.py
+++ b/src/tests/intg/ldap_test.py
@@ -174,7 +174,6 @@ def cleanup_sssd_process():
             time.sleep(1)
     except:
         pass
-    subprocess.call(["sss_cache", "-E"])
     for path in os.listdir(config.DB_PATH):
         os.unlink(config.DB_PATH + "/" + path)
     for path in os.listdir(config.MCACHE_PATH):
diff --git a/src/tests/intg/test_local_domain.py b/src/tests/intg/test_local_domain.py
index 4de0f5da2da38b71fef07d0a993fc4beaca39d9a..31834b5b9ab93329ac5eb76e3676d6f89f5b996e 100644
--- a/src/tests/intg/test_local_domain.py
+++ b/src/tests/intg/test_local_domain.py
@@ -58,7 +58,6 @@ def create_sssd_fixture(request):
             stop_sssd()
         except:
             pass
-        subprocess.call(["sss_cache", "-E"])
         for path in os.listdir(config.DB_PATH):
             os.unlink(config.DB_PATH + "/" + path)
         for path in os.listdir(config.MCACHE_PATH):
diff --git a/src/tests/intg/test_memory_cache.py b/src/tests/intg/test_memory_cache.py
index d324f29fb1ddcf05a31a7eb22668234ece8e7301..70e2357c7a259651ace283f2f792a4733d704380 100644
--- a/src/tests/intg/test_memory_cache.py
+++ b/src/tests/intg/test_memory_cache.py
@@ -100,7 +100,6 @@ def create_sssd_fixture(request):
             stop_sssd()
         except:
             pass
-        subprocess.call(["sss_cache", "-E"])
         for path in os.listdir(config.DB_PATH):
             os.unlink(config.DB_PATH + "/" + path)
         for path in os.listdir(config.MCACHE_PATH):
-- 
2.5.0



More information about the sssd-devel mailing list