[SSSD] [PATCH] sss_client: Re-check memcache after acquiring the lock
Lukas Slebodnik
lslebodn at redhat.com
Tue Jun 30 21:12:40 UTC 2015
ehlo,
patches for ticket #2581 are attached.
They depends on previous mmap patches because also initgroups
memory chace should be rechecked after acquiring the lock.
LS
-------------- next part --------------
>From b01c1bb88cf969537e13308a2bc4f5b9c0373b2d Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Tue, 30 Jun 2015 20:19:42 +0200
Subject: [PATCH 1/2] sss_client: Use unique lock for memory cache
Previously the sma lock was used as for communication with
responder. However it would cause a deadlock in case of
re-checking memcache after acquiring the lock and before communication with
responder..
Required by:
https://fedorahosted.org/sssd/ticket/2581
---
src/sss_client/common.c | 20 ++++++++++++++++++++
src/sss_client/nss_mc_common.c | 8 ++++----
src/sss_client/sss_cli.h | 2 ++
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 383572bada613e745ed198de2260a7fd27e43544..970cfe6f31470c95af8ba569ee403cb292a99c69 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -1032,6 +1032,7 @@ struct sss_mutex {
static void sss_nss_mt_init(void);
static void sss_pam_mt_init(void);
+static void sss_nss_mc_mt_init(void);
static struct sss_mutex sss_nss_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER,
.once = PTHREAD_ONCE_INIT,
@@ -1041,6 +1042,9 @@ static struct sss_mutex sss_pam_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER,
.once = PTHREAD_ONCE_INIT,
.init = sss_pam_mt_init };
+static struct sss_mutex sss_nss_mc_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER,
+ .once = PTHREAD_ONCE_INIT,
+ .init = sss_nss_mc_mt_init };
/* Wrappers for robust mutex support */
static int sss_mutexattr_setrobust (pthread_mutexattr_t *attr)
@@ -1125,6 +1129,20 @@ void sss_pam_unlock(void)
sss_mt_unlock(&sss_pam_mtx);
}
+/* NSS mutex wrappers */
+static void sss_nss_mc_mt_init(void)
+{
+ sss_mt_init(&sss_nss_mc_mtx);
+}
+void sss_nss_mc_lock(void)
+{
+ sss_mt_lock(&sss_nss_mc_mtx);
+}
+void sss_nss_mc_unlock(void)
+{
+ sss_mt_unlock(&sss_nss_mc_mtx);
+}
+
#else
/* sorry no mutexes available */
@@ -1132,6 +1150,8 @@ void sss_nss_lock(void) { return; }
void sss_nss_unlock(void) { return; }
void sss_pam_lock(void) { return; }
void sss_pam_unlock(void) { return; }
+void sss_nss_mc_lock(void) { return; }
+void sss_nss_mc_unlock(void) { return; }
#endif
diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
index 89ff6b46e2abee03039cfd632ef50231eab92eec..707d1249fe14812897a69ecf8d12a141d2fb5ed0 100644
--- a/src/sss_client/nss_mc_common.c
+++ b/src/sss_client/nss_mc_common.c
@@ -121,7 +121,7 @@ static errno_t sss_nss_mc_init_ctx(const char *name,
char *file = NULL;
int ret;
- sss_nss_lock();
+ sss_nss_mc_lock();
/* check if ctx is initialised by previous thread. */
if (ctx->initialized != UNINITIALIZED) {
ret = sss_nss_check_header(ctx);
@@ -172,7 +172,7 @@ done:
sss_nss_mc_destroy_ctx(ctx);
}
free(file);
- sss_nss_unlock();
+ sss_nss_mc_unlock();
return ret;
}
@@ -217,11 +217,11 @@ errno_t sss_nss_mc_get_ctx(const char *name, struct sss_cli_mc_ctx *ctx)
}
if (ctx->initialized == RECYCLED && ctx->active_threads == 0) {
/* just one thread should call munmap */
- sss_nss_lock();
+ sss_nss_mc_lock();
if (ctx->initialized == RECYCLED) {
sss_nss_mc_destroy_ctx(ctx);
}
- sss_nss_unlock();
+ sss_nss_mc_unlock();
}
if (need_decrement) {
/* In case of error, we will not touch mmapped area => decrement */
diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h
index 6902d711e30ef327f1c7bcf8348ff419d3f63092..0dfb525bacba5f6928e8ece76e05f60d7f2eebd5 100644
--- a/src/sss_client/sss_cli.h
+++ b/src/sss_client/sss_cli.h
@@ -594,6 +594,8 @@ void sss_nss_lock(void);
void sss_nss_unlock(void);
void sss_pam_lock(void);
void sss_pam_unlock(void);
+void sss_nss_mc_lock(void);
+void sss_nss_mc_unlock(void);
errno_t sss_readrep_copy_string(const char *in,
size_t *offset,
--
2.4.3
-------------- next part --------------
>From b916fadf10140f4d9a60460d6923f0fb9938df78 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Tue, 30 Jun 2015 18:56:38 +0200
Subject: [PATCH 2/2] sss_client: Re-check memcache after acquiring the lock
Resolves:
https://fedorahosted.org/sssd/ticket/2581
---
src/sss_client/nss_group.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
src/sss_client/nss_passwd.c | 42 +++++++++++++++++++++++++++++
2 files changed, 106 insertions(+)
diff --git a/src/sss_client/nss_group.c b/src/sss_client/nss_group.c
index b614fcf7f493345f793d3d4a28df998534ca546b..0e686af43aeb84a5938315e3922e9fcf2fef4e83 100644
--- a/src/sss_client/nss_group.c
+++ b/src/sss_client/nss_group.c
@@ -316,6 +316,28 @@ enum nss_status _nss_sss_initgroups_dyn(const char *user, gid_t group,
sss_nss_lock();
+ /* previous thread might already initialize entry in mmap cache */
+ ret = sss_nss_mc_initgroups_dyn(user, user_len, group, start, size,
+ groups, limit);
+ switch (ret) {
+ case 0:
+ *errnop = 0;
+ nret = NSS_STATUS_SUCCESS;
+ goto out;
+ case ERANGE:
+ *errnop = ERANGE;
+ nret = NSS_STATUS_TRYAGAIN;
+ goto out;
+ case ENOENT:
+ /* fall through, we need to actively ask the parent
+ * if no entry is found */
+ break;
+ default:
+ /* if using the mmaped cache failed,
+ * fall back to socket based comms */
+ break;
+ }
+
nret = sss_nss_make_request(SSS_NSS_INITGR, &rd,
&repbuf, &replen, errnop);
if (nret != NSS_STATUS_SUCCESS) {
@@ -418,6 +440,27 @@ enum nss_status _nss_sss_getgrnam_r(const char *name, struct group *result,
sss_nss_lock();
+ /* previous thread might already initialize entry in mmap cache */
+ ret = sss_nss_mc_getgrnam(name, name_len, result, buffer, buflen);
+ switch (ret) {
+ case 0:
+ *errnop = 0;
+ nret = NSS_STATUS_SUCCESS;
+ goto out;
+ case ERANGE:
+ *errnop = ERANGE;
+ nret = NSS_STATUS_TRYAGAIN;
+ goto out;
+ case ENOENT:
+ /* fall through, we need to actively ask the parent
+ * if no entry is found */
+ break;
+ default:
+ /* if using the mmaped cache failed,
+ * fall back to socket based comms */
+ break;
+ }
+
nret = sss_nss_get_getgr_cache(name, 0, GETGR_NAME,
&repbuf, &replen, errnop);
if (nret == NSS_STATUS_NOTFOUND) {
@@ -509,6 +552,27 @@ enum nss_status _nss_sss_getgrgid_r(gid_t gid, struct group *result,
sss_nss_lock();
+ /* previous thread might already initialize entry in mmap cache */
+ ret = sss_nss_mc_getgrgid(gid, result, buffer, buflen);
+ switch (ret) {
+ case 0:
+ *errnop = 0;
+ nret = NSS_STATUS_SUCCESS;
+ goto out;
+ case ERANGE:
+ *errnop = ERANGE;
+ nret = NSS_STATUS_TRYAGAIN;
+ goto out;
+ case ENOENT:
+ /* fall through, we need to actively ask the parent
+ * if no entry is found */
+ break;
+ default:
+ /* if using the mmaped cache failed,
+ * fall back to socket based comms */
+ break;
+ }
+
nret = sss_nss_get_getgr_cache(NULL, gid, GETGR_GID,
&repbuf, &replen, errnop);
if (nret == NSS_STATUS_NOTFOUND) {
diff --git a/src/sss_client/nss_passwd.c b/src/sss_client/nss_passwd.c
index 290aed80e0ddd817a0b2c818fdd0816cd2673a41..c43f9bc50f43599b541e97f5a5aa60de036a5cdf 100644
--- a/src/sss_client/nss_passwd.c
+++ b/src/sss_client/nss_passwd.c
@@ -178,6 +178,27 @@ enum nss_status _nss_sss_getpwnam_r(const char *name, struct passwd *result,
sss_nss_lock();
+ /* previous thread might already initialize entry in mmap cache */
+ ret = sss_nss_mc_getpwnam(name, name_len, result, buffer, buflen);
+ switch (ret) {
+ case 0:
+ *errnop = 0;
+ nret = NSS_STATUS_SUCCESS;
+ goto out;
+ case ERANGE:
+ *errnop = ERANGE;
+ nret = NSS_STATUS_TRYAGAIN;
+ goto out;
+ case ENOENT:
+ /* fall through, we need to actively ask the parent
+ * if no entry is found */
+ break;
+ default:
+ /* if using the mmaped cache failed,
+ * fall back to socket based comms */
+ break;
+ }
+
nret = sss_nss_make_request(SSS_NSS_GETPWNAM, &rd,
&repbuf, &replen, errnop);
if (nret != NSS_STATUS_SUCCESS) {
@@ -261,6 +282,27 @@ enum nss_status _nss_sss_getpwuid_r(uid_t uid, struct passwd *result,
sss_nss_lock();
+ /* previous thread might already initialize entry in mmap cache */
+ ret = sss_nss_mc_getpwuid(uid, result, buffer, buflen);
+ switch (ret) {
+ case 0:
+ *errnop = 0;
+ nret = NSS_STATUS_SUCCESS;
+ goto out;
+ case ERANGE:
+ *errnop = ERANGE;
+ nret = NSS_STATUS_TRYAGAIN;
+ goto out;
+ case ENOENT:
+ /* fall through, we need to actively ask the parent
+ * if no entry is found */
+ break;
+ default:
+ /* if using the mmaped cache failed,
+ * fall back to socket based comms */
+ break;
+ }
+
nret = sss_nss_make_request(SSS_NSS_GETPWUID, &rd,
&repbuf, &replen, errnop);
if (nret != NSS_STATUS_SUCCESS) {
--
2.4.3
More information about the sssd-devel
mailing list