>From 79e23d5ad87ec454fe2f3f2b7e79694897db10bd Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Mon, 14 Jul 2014 16:02:22 +0200 Subject: [PATCH 1/3] sss_client: thread safe initialisation of sss_nss_mc_get_ctx In multi threaded application, it may happen that more threads will call function getpwuid(or similar) and sss client will not have initialized structure for fast memory cache. This structure is initialized just once. There isn't any problem with multi threaded application after successful initialisation. The race condition will happen if more threads try to initialise structure sss_cli_mc_ctx in function sss_nss_mc_get_ctx (ctx->initialized is false) It takes some time to initialise mmap cache: open file, get file size, mmap file, initialize structure sss_cli_mc_ctx. One of problems is that file with memory cache can be opened more times (file descriptor leak), but the race condition is with initialising structure sss_cli_mc_ctx. One tread will start to initialise this structure; another thread will think that structure is already initialised and will check consistency of this structure. It will fail because 1st thread did not finish initialisation. Therefore 2nd thread will return EINVAL and will do clean up in done section: munmap, close file and reset structure data. The 1st thread will finish an try to use memory cache, but structure was zero initialised by 2nd thread and it will cause dereference of NULL pointer in 1st thread (SIGSEGV) or dividing by zero in murmurhash function(SIGFPE) This patch does not allow to parallel initialisation. Resolves: https://fedorahosted.org/sssd/ticket/2380 --- src/sss_client/nss_mc.h | 1 + src/sss_client/nss_mc_common.c | 10 ++++++++++ src/sss_client/nss_mc_group.c | 3 ++- src/sss_client/nss_mc_passwd.c | 3 ++- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/sss_client/nss_mc.h b/src/sss_client/nss_mc.h index 685cc41c0530750d890050f0917dc88be14d96ea..a77b86261bfbafc83b0e65315ec478f7f40b57e8 100644 --- a/src/sss_client/nss_mc.h +++ b/src/sss_client/nss_mc.h @@ -37,6 +37,7 @@ typedef int errno_t; struct sss_cli_mc_ctx { bool initialized; int fd; + volatile int init_count; /* numbers of threads trying to initialize ctx */ uint32_t seed; /* seed from the tables header */ diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c index db9be94b442b1b5a97ff9074fb1d98a1feea5e1a..d918b697f9cf159e6dbea46ea0f670993dc14bbf 100644 --- a/src/sss_client/nss_mc_common.c +++ b/src/sss_client/nss_mc_common.c @@ -118,6 +118,15 @@ errno_t sss_nss_mc_get_ctx(const char *name, struct sss_cli_mc_ctx *ctx) goto done; } + if (__sync_add_and_fetch(&ctx->init_count, 1) != 1) { + /* + * More threads try to init sss_cli_mc_ctx. There can be raise + * condition and we do not want to call clean up in case of error + * in done section. Therefore return EAGAIN is used. + */ + return EAGAIN; + } + ret = asprintf(&file, "%s/%s", SSS_NSS_MCACHE_DIR, name); if (ret == -1) { ret = ENOMEM; @@ -154,6 +163,7 @@ errno_t sss_nss_mc_get_ctx(const char *name, struct sss_cli_mc_ctx *ctx) } ctx->initialized = true; + ctx->init_count = 0; ret = 0; diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c index 5af55468f1a6fb2847844f2131cad6e2f83588b4..7a3788e1d853c197661b220b67434f2c2275c9a8 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 = { false, -1, 0, + 0, NULL, 0, NULL, 0, NULL, 0 }; static errno_t sss_nss_mc_parse_result(struct sss_mc_rec *rec, struct group *result, diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c index 95b8a0407bc8650d6354b13823fbb486b8c64839..f2970eefa3f52601eca3836280aa24173def296c 100644 --- a/src/sss_client/nss_mc_passwd.c +++ b/src/sss_client/nss_mc_passwd.c @@ -28,7 +28,8 @@ #include #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 = { false, -1, 0, + 0, NULL, 0, NULL, 0, NULL, 0 }; static errno_t sss_nss_mc_parse_result(struct sss_mc_rec *rec, struct passwd *result, -- 1.9.3