This commit is contained in:
Jakub Hrozek 2013-08-23 15:41:45 +02:00
parent a35bab9380
commit 413e09fdbc
9 changed files with 948 additions and 1 deletions

View File

@ -0,0 +1,55 @@
From 545f49b72cdf8453fb0b85c9d87e7d4711da57da Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn@redhat.com>
Date: Wed, 14 Aug 2013 13:21:31 +0200
Subject: [PATCH] proxy: Alocate auth tokens in struct authtok_conv
Struct sss_auth_token became opaque in commit
9acfb09f7969a69f58bd45c856b01700541853ca.
All ocasions of "struct sss_auth_token" was replaced with pointer to this
struct, but proper initialization of auth_tokens was missing
in struct authtok_conv.
Resolves:
https://fedorahosted.org/sssd/ticket/2046
diff --git a/src/providers/proxy/proxy_child.c b/src/providers/proxy/proxy_child.c
index efdf9120a8ec529283aa3f2c6cacd98d659a6ef7..6f95ede6af78f0286933fc4259dd258ef69f2d37 100644
--- a/src/providers/proxy/proxy_child.c
+++ b/src/providers/proxy/proxy_child.c
@@ -201,6 +201,23 @@ static errno_t call_pam_stack(const char *pam_target, struct pam_data *pd)
conv.conv=proxy_internal_conv;
}
auth_data = talloc_zero(pd, struct authtok_conv);
+ if (auth_data == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_zero failed.\n"));
+ return ENOMEM;
+ }
+ auth_data->authtok = sss_authtok_new(auth_data);
+ if (auth_data->authtok == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("sss_authtok_new failed.\n"));
+ ret = ENOMEM;
+ goto fail;
+ }
+ auth_data->newauthtok = sss_authtok_new(auth_data);
+ if (auth_data->newauthtok == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("sss_authtok_new failed.\n"));
+ ret = ENOMEM;
+ goto fail;
+ }
+
conv.appdata_ptr=auth_data;
ret = pam_start(pam_target, pd->user, &conv, &pamh);
@@ -279,6 +296,9 @@ static errno_t call_pam_stack(const char *pam_target, struct pam_data *pd)
pd->pam_status = pam_status;
return EOK;
+fail:
+ talloc_free(auth_data);
+ return ret;
}
static int pc_pam_handler(DBusMessage *message, struct sbus_connection *conn)
--
1.8.3.1

View File

@ -0,0 +1,196 @@
From 9028706a00da1bc48547e74aa872c825ac15adb2 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzidek@redhat.com>
Date: Mon, 5 Aug 2013 20:59:33 +0200
Subject: [PATCH] mmap_cache: Check if slot and name_ptr are not invalid.
This patch prevents jumping outside of allocated memory in
case of corrupted slot or name_ptr values. It is not proper
solution, just hotfix until we find out what is the root cause
of ticket https://fedorahosted.org/sssd/ticket/2018
---
src/responder/nss/nsssrv_mmap_cache.c | 54 +++++++++++++++++++++++++++++++++--
src/responder/nss/nsssrv_mmap_cache.h | 2 ++
src/sss_client/nss_mc_group.c | 8 ++++++
src/sss_client/nss_mc_passwd.c | 8 ++++++
src/util/mmap_cache.h | 3 ++
5 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index 49878fcfb91c195fb864c83e1da0163a23dbe5a7..cd5a6436e005b4c7f5622eaff2f259de3bbe5d29 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -373,8 +373,23 @@ static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
}
while (slot != MC_INVALID_VAL) {
+ if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ ("Corrupted fastcache. Slot number too big.\n"));
+ sss_mmap_cache_reset(mcc);
+ return NULL;
+ }
+
rec = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec);
name_ptr = *((rel_ptr_t *)rec->data);
+ /* FIXME: This check relies on fact that offset of member strs
+ * is the same in structures sss_mc_pwd_data and sss_mc_group_data. */
+ if (name_ptr != offsetof(struct sss_mc_pwd_data, strs)) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ ("Corrupted fastcache. name_ptr value is %u.\n", name_ptr));
+ sss_mmap_cache_reset(mcc);
+ return NULL;
+ }
t_key = (char *)rec->data + name_ptr;
if (strcmp(key->str, t_key) == 0) {
@@ -608,6 +623,13 @@ errno_t sss_mmap_cache_pw_invalidate_uid(struct sss_mc_ctx *mcc, uid_t uid)
}
while (slot != MC_INVALID_VAL) {
+ if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+ DEBUG(SSSDBG_FATAL_FAILURE, ("Corrupted fastcache.\n"));
+ sss_mmap_cache_reset(mcc);
+ ret = ENOENT;
+ goto done;
+ }
+
rec = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec);
data = (struct sss_mc_pwd_data *)(&rec->data);
@@ -739,6 +761,13 @@ errno_t sss_mmap_cache_gr_invalidate_gid(struct sss_mc_ctx *mcc, gid_t gid)
}
while (slot != MC_INVALID_VAL) {
+ if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+ DEBUG(SSSDBG_FATAL_FAILURE, ("Corrupted fastcache.\n"));
+ sss_mmap_cache_reset(mcc);
+ ret = ENOENT;
+ goto done;
+ }
+
rec = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec);
data = (struct sss_mc_grp_data *)(&rec->data);
@@ -889,8 +918,9 @@ static void sss_mc_header_update(struct sss_mc_ctx *mc_ctx, int status)
/* update header using barriers */
h = (struct sss_mc_header *)mc_ctx->mmap_base;
MC_RAISE_BARRIER(h);
- if (status != SSS_MC_HEADER_RECYCLED) {
- /* no reason to update anything else if the file is recycled */
+ if (status == SSS_MC_HEADER_ALIVE) {
+ /* no reason to update anything else if the file is recycled or
+ * right before reset */
h->hash_table = MC_PTR_DIFF(mc_ctx->hash_table, mc_ctx->mmap_base);
h->free_table = MC_PTR_DIFF(mc_ctx->free_table, mc_ctx->mmap_base);
h->data_table = MC_PTR_DIFF(mc_ctx->data_table, mc_ctx->mmap_base);
@@ -1113,3 +1143,23 @@ done:
talloc_free(tmp_ctx);
return ret;
}
+
+/* Erase all contents of the mmap cache. This will bring the cache
+ * to the same state as if it was just initialized. */
+void sss_mmap_cache_reset(struct sss_mc_ctx *mc_ctx)
+{
+ if (mc_ctx == NULL) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ ("Fastcache not initialized. Nothing to do.\n"));
+ return;
+ }
+
+ sss_mc_header_update(mc_ctx, SSS_MC_HEADER_UNINIT);
+
+ /* Reset the mmaped area */
+ memset(mc_ctx->data_table, 0xff, mc_ctx->dt_size);
+ memset(mc_ctx->free_table, 0x00, mc_ctx->ft_size);
+ memset(mc_ctx->hash_table, 0xff, mc_ctx->ht_size);
+
+ sss_mc_header_update(mc_ctx, SSS_MC_HEADER_ALIVE);
+}
diff --git a/src/responder/nss/nsssrv_mmap_cache.h b/src/responder/nss/nsssrv_mmap_cache.h
index 25cec40cc26d6732c1465c014ab5aab4a59ec906..fdeaa09126858b04958f4e3eee17b8a92cf61350 100644
--- a/src/responder/nss/nsssrv_mmap_cache.h
+++ b/src/responder/nss/nsssrv_mmap_cache.h
@@ -63,4 +63,6 @@ errno_t sss_mmap_cache_gr_invalidate_gid(struct sss_mc_ctx *mcc, gid_t gid);
errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, size_t n_elem,
time_t timeout, struct sss_mc_ctx **mc_ctx);
+void sss_mmap_cache_reset(struct sss_mc_ctx *mc_ctx);
+
#endif /* _NSSSRV_MMAP_CACHE_H_ */
diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index b3e9a8a0bd6b0a90783d60c8809bb1f542b31f54..2d69be93b76587a7e474c1db55430930ca850321 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -116,6 +116,10 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
}
while (slot != MC_INVALID_VAL) {
+ if (slot > MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
+ /* This probably means that the memory cache was corrupted. */
+ return ENOENT;
+ }
ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
if (ret) {
@@ -180,6 +184,10 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
}
while (slot != MC_INVALID_VAL) {
+ if (slot > MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
+ /* This probably means that the memory cache was corrupted. */
+ return ENOENT;
+ }
ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
if (ret) {
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index 4acc6425e2609c140d71a1013ca203d7db074e02..fa21bd2896a1de868735cd6d22d09159fd3d8ed2 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -117,6 +117,10 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
}
while (slot != MC_INVALID_VAL) {
+ if (slot > MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
+ /* This probably means that the memory cache was corrupted */
+ return ENOENT;
+ }
ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
if (ret) {
@@ -181,6 +185,10 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
}
while (slot != MC_INVALID_VAL) {
+ if (slot > MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
+ /* This probably means that the memory cache was corrupted */
+ return ENOENT;
+ }
ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
if (ret) {
diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h
index 55383c056b36460676958754baada9c294331b5e..6c223df6c79b5ac10786903eecd1cb8c8a3999a5 100644
--- a/src/util/mmap_cache.h
+++ b/src/util/mmap_cache.h
@@ -78,6 +78,7 @@ typedef uint32_t rel_ptr_t;
#define SSS_MC_MAJOR_VNO 0
#define SSS_MC_MINOR_VNO 4
+#define SSS_MC_HEADER_UNINIT 0 /* after ftruncate or before reset */
#define SSS_MC_HEADER_ALIVE 1 /* current and in use */
#define SSS_MC_HEADER_RECYCLED 2 /* file was recycled, reopen asap */
@@ -109,6 +110,8 @@ struct sss_mc_rec {
char data[0];
};
+/* FIXME: Function sss_mc_find_record currently relies on fact that
+ * offset of strs is the same in both sss_mc_pwd_data and sss_mc_grp_data. */
struct sss_mc_pwd_data {
rel_ptr_t name; /* ptr to name string, rel. to struct base addr */
uint32_t uid;
--
1.8.3.1

View File

@ -0,0 +1,119 @@
From 8a5931bcc8e9034e4beb92fc9addf3f7fcf83fd6 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzidek@redhat.com>
Date: Mon, 12 Aug 2013 19:29:56 +0200
Subject: [PATCH 1/4] mmap_cache: Check data->name value in client code
data->name value must be checked to prevent segfaults in
case of corrupted memory cache.
resolves:
https://fedorahosted.org/sssd/ticket/2018
---
src/sss_client/nss_mc_group.c | 18 ++++++++++++++++++
src/sss_client/nss_mc_passwd.c | 19 +++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index 2d69be93b76587a7e474c1db55430930ca850321..da5da0411e556c30c4a3db6faf80139d65ae817c 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -23,6 +23,7 @@
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
+#include <stddef.h>
#include <sys/mman.h>
#include <time.h>
#include "nss_mc.h"
@@ -102,12 +103,17 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
uint32_t hash;
uint32_t slot;
int ret;
+ size_t strs_offset;
+ uint8_t *max_addr;
ret = sss_nss_mc_get_ctx("group", &gr_mc_ctx);
if (ret) {
return ret;
}
+ /* Get max address of data table. */
+ max_addr = gr_mc_ctx.data_table + gr_mc_ctx.dt_size;
+
/* hashes are calculated including the NULL terminator */
hash = sss_nss_mc_hash(&gr_mc_ctx, name, name_len + 1);
slot = gr_mc_ctx.hash_table[hash];
@@ -133,7 +139,19 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
continue;
}
+ strs_offset = offsetof(struct sss_mc_grp_data, strs);
data = (struct sss_mc_grp_data *)rec->data;
+ /* Integrity check
+ * - name_len cannot be longer than all strings
+ * - data->name cannot point outside strings
+ * - all strings must be within data_table */
+ if (name_len > data->strs_len
+ || (data->name + name_len) > (strs_offset + data->strs_len)
+ || (uint8_t *)data->strs + data->strs_len > max_addr) {
+ ret = ENOENT;
+ goto done;
+ }
+
rec_name = (char *)data + data->name;
if (strcmp(name, rec_name) == 0) {
break;
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index fa21bd2896a1de868735cd6d22d09159fd3d8ed2..4b08766857d5013e6f13c3dbe574c5a88fa915b0 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -23,6 +23,7 @@
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
+#include <stddef.h>
#include <sys/mman.h>
#include <time.h>
#include "nss_mc.h"
@@ -103,12 +104,17 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
uint32_t hash;
uint32_t slot;
int ret;
+ size_t strs_offset;
+ uint8_t *max_addr;
ret = sss_nss_mc_get_ctx("passwd", &pw_mc_ctx);
if (ret) {
return ret;
}
+ /* Get max address of data table. */
+ max_addr = pw_mc_ctx.data_table + pw_mc_ctx.dt_size;
+
/* hashes are calculated including the NULL terminator */
hash = sss_nss_mc_hash(&pw_mc_ctx, name, name_len + 1);
slot = pw_mc_ctx.hash_table[hash];
@@ -134,7 +140,20 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
continue;
}
+ strs_offset = offsetof(struct sss_mc_pwd_data, strs);
+
data = (struct sss_mc_pwd_data *)rec->data;
+ /* Integrity check
+ * - name_len cannot be longer than all strings
+ * - data->name cannot point outside strings
+ * - all strings must be within data_table */
+ if (name_len > data->strs_len
+ || (data->name + name_len) > (strs_offset + data->strs_len)
+ || (uint8_t *)data->strs + data->strs_len > max_addr) {
+ ret = ENOENT;
+ goto done;
+ }
+
rec_name = (char *)data + data->name;
if (strcmp(name, rec_name) == 0) {
break;
--
1.8.3.1

View File

@ -0,0 +1,143 @@
From e61044d99ce1e68057fda236f04a731f1f3f299a Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzidek@redhat.com>
Date: Wed, 14 Aug 2013 18:01:30 +0200
Subject: [PATCH 2/4] mmap_cache: Remove triple checks in client code.
We had pattern in client code with 3 conditions
that can be replaced with one.
---
src/sss_client/nss_mc_group.c | 30 ++++++++++--------------------
src/sss_client/nss_mc_passwd.c | 30 ++++++++++--------------------
2 files changed, 20 insertions(+), 40 deletions(-)
diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index da5da0411e556c30c4a3db6faf80139d65ae817c..9fe72a60e58e0c94f0c38f243276060b70f28aa9 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -117,16 +117,11 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
/* hashes are calculated including the NULL terminator */
hash = sss_nss_mc_hash(&gr_mc_ctx, name, name_len + 1);
slot = gr_mc_ctx.hash_table[hash];
- if (slot > MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
- return ENOENT;
- }
-
- while (slot != MC_INVALID_VAL) {
- if (slot > MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
- /* This probably means that the memory cache was corrupted. */
- return ENOENT;
- }
+ /* If slot is not within the bounds of mmaped region and
+ * it's value is not MC_INVALID_VAL, then the cache is
+ * probbably corrupted. */
+ while (slot < MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
if (ret) {
goto done;
@@ -160,7 +155,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
slot = rec->next;
}
- if (slot == MC_INVALID_VAL) {
+ if (slot >= MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
ret = ENOENT;
goto done;
}
@@ -197,16 +192,11 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
/* hashes are calculated including the NULL terminator */
hash = sss_nss_mc_hash(&gr_mc_ctx, gidstr, len+1);
slot = gr_mc_ctx.hash_table[hash];
- if (slot > MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
- return ENOENT;
- }
-
- while (slot != MC_INVALID_VAL) {
- if (slot > MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
- /* This probably means that the memory cache was corrupted. */
- return ENOENT;
- }
+ /* If slot is not within the bounds of mmaped region and
+ * it's value is not MC_INVALID_VAL, then the cache is
+ * probbably corrupted. */
+ while (slot < MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
if (ret) {
goto done;
@@ -227,7 +217,7 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
slot = rec->next;
}
- if (slot == MC_INVALID_VAL) {
+ if (slot >= MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
ret = ENOENT;
goto done;
}
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index 4b08766857d5013e6f13c3dbe574c5a88fa915b0..7aca4a04b6b19b50b883960229083b688639ee4f 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -118,16 +118,11 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
/* hashes are calculated including the NULL terminator */
hash = sss_nss_mc_hash(&pw_mc_ctx, name, name_len + 1);
slot = pw_mc_ctx.hash_table[hash];
- if (slot > MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
- return ENOENT;
- }
-
- while (slot != MC_INVALID_VAL) {
- if (slot > MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
- /* This probably means that the memory cache was corrupted */
- return ENOENT;
- }
+ /* If slot is not within the bounds of mmaped region and
+ * it's value is not MC_INVALID_VAL, then the cache is
+ * probbably corrupted. */
+ while (slot < MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
if (ret) {
goto done;
@@ -162,7 +157,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
slot = rec->next;
}
- if (slot == MC_INVALID_VAL) {
+ if (slot >= MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
ret = ENOENT;
goto done;
}
@@ -199,16 +194,11 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
/* hashes are calculated including the NULL terminator */
hash = sss_nss_mc_hash(&pw_mc_ctx, uidstr, len+1);
slot = pw_mc_ctx.hash_table[hash];
- if (slot > MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
- return ENOENT;
- }
-
- while (slot != MC_INVALID_VAL) {
- if (slot > MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
- /* This probably means that the memory cache was corrupted */
- return ENOENT;
- }
+ /* If slot is not within the bounds of mmaped region and
+ * it's value is not MC_INVALID_VAL, then the cache is
+ * probbably corrupted. */
+ while (slot < MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
if (ret) {
goto done;
@@ -229,7 +219,7 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
slot = rec->next;
}
- if (slot == MC_INVALID_VAL) {
+ if (slot >= MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
ret = ENOENT;
goto done;
}
--
1.8.3.1

View File

@ -0,0 +1,162 @@
From 13df7b9e400211c717284fb841c849ba034ed348 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzidek@redhat.com>
Date: Wed, 14 Aug 2013 18:22:06 +0200
Subject: [PATCH 3/4] mmap_cache: Off by one error.
Removes off by one error when using macro MC_SIZE_TO_SLOTS
and adds new macro MC_SLOT_WITHIN_BOUNDS.
---
src/responder/nss/nsssrv_mmap_cache.c | 12 ++++++------
src/sss_client/nss_mc_group.c | 8 ++++----
src/sss_client/nss_mc_passwd.c | 8 ++++----
src/util/mmap_cache.h | 3 +++
4 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index cd5a6436e005b4c7f5622eaff2f259de3bbe5d29..a1bab0c8d877a354451ad5c31ec5e86b294837e9 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -368,12 +368,12 @@ static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
hash = sss_mc_hash(mcc, key->str, key->len);
slot = mcc->hash_table[hash];
- if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
return NULL;
}
while (slot != MC_INVALID_VAL) {
- if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
DEBUG(SSSDBG_FATAL_FAILURE,
("Corrupted fastcache. Slot number too big.\n"));
sss_mmap_cache_reset(mcc);
@@ -617,13 +617,13 @@ errno_t sss_mmap_cache_pw_invalidate_uid(struct sss_mc_ctx *mcc, uid_t uid)
hash = sss_mc_hash(mcc, uidstr, strlen(uidstr) + 1);
slot = mcc->hash_table[hash];
- if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
ret = ENOENT;
goto done;
}
while (slot != MC_INVALID_VAL) {
- if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
DEBUG(SSSDBG_FATAL_FAILURE, ("Corrupted fastcache.\n"));
sss_mmap_cache_reset(mcc);
ret = ENOENT;
@@ -755,13 +755,13 @@ errno_t sss_mmap_cache_gr_invalidate_gid(struct sss_mc_ctx *mcc, gid_t gid)
hash = sss_mc_hash(mcc, gidstr, strlen(gidstr) + 1);
slot = mcc->hash_table[hash];
- if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
ret = ENOENT;
goto done;
}
while (slot != MC_INVALID_VAL) {
- if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
DEBUG(SSSDBG_FATAL_FAILURE, ("Corrupted fastcache.\n"));
sss_mmap_cache_reset(mcc);
ret = ENOENT;
diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index 9fe72a60e58e0c94f0c38f243276060b70f28aa9..4e3d9fb0dfffc2194a6d1e2035ed5782af528fce 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -121,7 +121,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
/* If slot is not within the bounds of mmaped region and
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
- while (slot < MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
+ while (MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
if (ret) {
goto done;
@@ -155,7 +155,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
slot = rec->next;
}
- if (slot >= MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
ret = ENOENT;
goto done;
}
@@ -196,7 +196,7 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
/* If slot is not within the bounds of mmaped region and
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
- while (slot < MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
+ while (MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
if (ret) {
goto done;
@@ -217,7 +217,7 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
slot = rec->next;
}
- if (slot >= MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
ret = ENOENT;
goto done;
}
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index 7aca4a04b6b19b50b883960229083b688639ee4f..a0a8d87f7475d8fea1bc32409a2d5c6af8f7896f 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -122,7 +122,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
/* If slot is not within the bounds of mmaped region and
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
- while (slot < MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
+ while (MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
if (ret) {
goto done;
@@ -157,7 +157,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
slot = rec->next;
}
- if (slot >= MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
ret = ENOENT;
goto done;
}
@@ -198,7 +198,7 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
/* If slot is not within the bounds of mmaped region and
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
- while (slot < MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
+ while (MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
if (ret) {
goto done;
@@ -219,7 +219,7 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
slot = rec->next;
}
- if (slot >= MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
ret = ENOENT;
goto done;
}
diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h
index 6c223df6c79b5ac10786903eecd1cb8c8a3999a5..abf8cac49dda1c53cf7aa2a428eefa49c42c4d8e 100644
--- a/src/util/mmap_cache.h
+++ b/src/util/mmap_cache.h
@@ -67,6 +67,9 @@ typedef uint32_t rel_ptr_t;
#define MC_SLOT_TO_PTR(base, slot, type) \
(type *)((base) + ((slot) * MC_SLOT_SIZE))
+#define MC_SLOT_WITHIN_BOUNDS(slot, dt_size) \
+ ((slot) < ((dt_size) / MC_SLOT_SIZE))
+
#define MC_VALID_BARRIER(val) (((val) & 0xff000000) == 0xf0000000)
#define MC_CHECK_RECORD_LENGTH(mc_ctx, rec) \
--
1.8.3.1

View File

@ -0,0 +1,118 @@
From 441e6050f4b67134d15862e401b4c4e8546d7387 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzidek@redhat.com>
Date: Thu, 15 Aug 2013 16:08:17 +0200
Subject: [PATCH 4/4] mmap_cache: Use better checks for corrupted mc in
responder
We introduced new way to check integrity of memcache in the
client code. We should use similiar checks in the responder.
---
src/responder/nss/nsssrv_mmap_cache.c | 56 +++++++++++++++++++++++++++++++++--
src/util/mmap_cache.h | 2 --
2 files changed, 53 insertions(+), 5 deletions(-)
diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index a1bab0c8d877a354451ad5c31ec5e86b294837e9..95a7fe9dc7dd527cab6261f68d1dd0ab2f738d2a 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -356,6 +356,39 @@ static errno_t sss_mc_find_free_slots(struct sss_mc_ctx *mcc,
return EOK;
}
+static errno_t sss_mc_get_strs_offset(struct sss_mc_ctx *mcc,
+ size_t *_offset)
+{
+ switch (mcc->type) {
+ case SSS_MC_PASSWD:
+ *_offset = offsetof(struct sss_mc_pwd_data, strs);
+ return EOK;
+ case SSS_MC_GROUP:
+ *_offset = offsetof(struct sss_mc_grp_data, strs);
+ return EOK;
+ default:
+ DEBUG(SSSDBG_FATAL_FAILURE, ("Unknown memory cache type.\n"));
+ return EINVAL;
+ }
+}
+
+static errno_t sss_mc_get_strs_len(struct sss_mc_ctx *mcc,
+ struct sss_mc_rec *rec,
+ size_t *_len)
+{
+ switch (mcc->type) {
+ case SSS_MC_PASSWD:
+ *_len = ((struct sss_mc_pwd_data *)&rec->data)->strs_len;
+ return EOK;
+ case SSS_MC_GROUP:
+ *_len = ((struct sss_mc_grp_data *)&rec->data)->strs_len;
+ return EOK;
+ default:
+ DEBUG(SSSDBG_FATAL_FAILURE, ("Unknown memory cache type.\n"));
+ return EINVAL;
+ }
+}
+
static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
struct sized_string *key)
{
@@ -364,6 +397,10 @@ static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
uint32_t slot;
rel_ptr_t name_ptr;
char *t_key;
+ size_t strs_offset;
+ size_t strs_len;
+ uint8_t *max_addr;
+ errno_t ret;
hash = sss_mc_hash(mcc, key->str, key->len);
@@ -372,6 +409,14 @@ static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
return NULL;
}
+ /* Get max address of data table. */
+ max_addr = mcc->data_table + mcc->dt_size;
+
+ ret = sss_mc_get_strs_offset(mcc, &strs_offset);
+ if (ret != EOK) {
+ return NULL;
+ }
+
while (slot != MC_INVALID_VAL) {
if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
DEBUG(SSSDBG_FATAL_FAILURE,
@@ -381,10 +426,15 @@ static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
}
rec = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec);
+ ret = sss_mc_get_strs_len(mcc, rec, &strs_len);
+ if (ret != EOK) {
+ return NULL;
+ }
+
name_ptr = *((rel_ptr_t *)rec->data);
- /* FIXME: This check relies on fact that offset of member strs
- * is the same in structures sss_mc_pwd_data and sss_mc_group_data. */
- if (name_ptr != offsetof(struct sss_mc_pwd_data, strs)) {
+ if (key->len > strs_len
+ || (name_ptr + key->len) > (strs_offset + strs_len)
+ || (uint8_t *)rec->data + strs_offset + strs_len > max_addr) {
DEBUG(SSSDBG_FATAL_FAILURE,
("Corrupted fastcache. name_ptr value is %u.\n", name_ptr));
sss_mmap_cache_reset(mcc);
diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h
index abf8cac49dda1c53cf7aa2a428eefa49c42c4d8e..7c6693ac8151ac488fc5f4d2384749784eb98c7d 100644
--- a/src/util/mmap_cache.h
+++ b/src/util/mmap_cache.h
@@ -113,8 +113,6 @@ struct sss_mc_rec {
char data[0];
};
-/* FIXME: Function sss_mc_find_record currently relies on fact that
- * offset of strs is the same in both sss_mc_pwd_data and sss_mc_grp_data. */
struct sss_mc_pwd_data {
rel_ptr_t name; /* ptr to name string, rel. to struct base addr */
uint32_t uid;
--
1.8.3.1

View File

@ -0,0 +1,98 @@
From 6e95ece6809a8a6c6b4fcddb6e076514dd835857 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn@redhat.com>
Date: Mon, 19 Aug 2013 05:39:28 +0200
Subject: [PATCH 1/2] mmap_cache: Skip records which doesn't have same hash
The code uses 2 hashes for each record, but only one hash table to
index them both, furthermore each record has only one single 'next'
pointer.
This means that in certain conditions a record main end up being on a
hash chain even though its hashes do not match the hash chain. This can
happen when another record 'drags' it in from another hash chain where
they both belong.
If the record without matching hashes happens to be the second of the
chain and the first record is removed, then the non matching record is
left on the wrong chain. On removal of the non-matching record the hash
chain will not be updated and the hash chain will end up pointing to an
invalid slot.
This slot may be later reused for another record and may not be the
first slot of this new record. In this case the hash chain will point to
arbitrary data and may cause issues if the slot is interpreted as the
head of a record.
By skipping any block that has no matching hashes upon removing the
first record in a chain we insure that dangling references cannot be
left in the hash table
Resolves:
https://fedorahosted.org/sssd/ticket/2049
---
src/responder/nss/nsssrv_mmap_cache.c | 36 +++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index fced018ebafb4224b3ea216ec99461538ea878da..522e6fa57640261ff4cc17bf554776d7d905bf7a 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -196,6 +196,27 @@ static void sss_mc_add_rec_to_chain(struct sss_mc_ctx *mcc,
cur->next = MC_PTR_TO_SLOT(mcc->data_table, rec);
}
+static inline uint32_t
+sss_mc_get_next_slot_with_hash(struct sss_mc_ctx *mcc,
+ struct sss_mc_rec *start_rec,
+ uint32_t hash)
+{
+ struct sss_mc_rec *rec;
+ uint32_t slot;
+
+ slot = start_rec->next;
+ while (slot != MC_INVALID_VAL) {
+ rec = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec);
+ if (rec->hash1 == hash || rec->hash2 == hash) {
+ break;
+ }
+
+ slot = rec->next;
+ }
+
+ return slot;
+}
+
static void sss_mc_rm_rec_from_chain(struct sss_mc_ctx *mcc,
struct sss_mc_rec *rec,
uint32_t hash)
@@ -213,7 +234,11 @@ static void sss_mc_rm_rec_from_chain(struct sss_mc_ctx *mcc,
slot = mcc->hash_table[hash];
cur = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec);
if (cur == rec) {
- mcc->hash_table[hash] = rec->next;
+ /* rec->next can refer to record without matching hashes.
+ * We need to skip this(those) records, because
+ * mcc->hash_table[hash] have to refer to valid start of the chain.
+ */
+ mcc->hash_table[hash] = sss_mc_get_next_slot_with_hash(mcc, rec, hash);
} else {
slot = cur->next;
while (slot != MC_INVALID_VAL) {
@@ -221,7 +246,14 @@ static void sss_mc_rm_rec_from_chain(struct sss_mc_ctx *mcc,
cur = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec);
if (cur == rec) {
/* changing a single uint32_t is atomic, so there is no
- * need to use barriers in this case */
+ * need to use barriers in this case.
+ *
+ * This situation is different to the removing record from
+ * the beggining of the chain. The record have to only be
+ * removed from chain, because this chain can be
+ * subset or supperset of another chain and we don't want
+ * to break another chains.
+ */
prev->next = cur->next;
slot = MC_INVALID_VAL;
} else {
--
1.8.3.1

View File

@ -0,0 +1,41 @@
From 6a6f9c3a48fbfd7c61cd0c6d0aa9b4bf7bdb5fd6 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn@redhat.com>
Date: Mon, 19 Aug 2013 07:24:46 +0200
Subject: [PATCH 2/2] mmap_cache: Use stricter check for hash keys.
ht_size is size of hash_table in bytes, but hash keys have type uint32_t
---
src/responder/nss/nsssrv_mmap_cache.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index 522e6fa57640261ff4cc17bf554776d7d905bf7a..c34997b8034d05796687ff7d380b367d5c7bba06 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -168,7 +168,7 @@ static void sss_mc_add_rec_to_chain(struct sss_mc_ctx *mcc,
struct sss_mc_rec *cur;
uint32_t slot;
- if (hash > mcc->ht_size) {
+ if (hash > MC_HT_ELEMS(mcc->ht_size)) {
/* Invalid hash. This should never happen, but better
* return than trying to access out of bounds memory */
return;
@@ -225,9 +225,11 @@ static void sss_mc_rm_rec_from_chain(struct sss_mc_ctx *mcc,
struct sss_mc_rec *cur = NULL;
uint32_t slot;
- if (hash > mcc->ht_size) {
- /* Invalid hash. This should never happen, but better
- * return than trying to access out of bounds memory */
+ if (hash > MC_HT_ELEMS(mcc->ht_size)) {
+ /* It can happen if rec->hash1 and rec->has2 was the same.
+ * or it is invalid hash. It is better to return
+ * than trying to access out of bounds memory
+ */
return;
}
--
1.8.3.1

View File

@ -8,7 +8,7 @@
Name: sssd Name: sssd
Version: 1.11.0 Version: 1.11.0
Release: 0.3.beta2%{?dist} Release: 0.4.beta2%{?dist}
Group: Applications/System Group: Applications/System
Summary: System Security Services Daemon Summary: System Security Services Daemon
License: GPLv3+ License: GPLv3+
@ -17,6 +17,15 @@ Source0: https://fedorahosted.org/released/sssd/%{name}-%{version}beta2.tar.gz
BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
### Patches ### ### Patches ###
Patch0001: 0001-proxy-Alocate-auth-tokens-in-struct-authtok_conv.patch
Patch0002: 0002-mmap_cache-Check-if-slot-and-name_ptr-are-not-invali.patch
Patch0003: 0003-mmap_cache-Check-data-name-value-in-client-code.patch
Patch0004: 0004-mmap_cache-Remove-triple-checks-in-client-code.patch
Patch0005: 0005-mmap_cache-Off-by-one-error.patch
Patch0006: 0006-mmap_cache-Use-better-checks-for-corrupted-mc-in-res.patch
Patch0007: 0007-mmap_cache-Skip-records-which-doesn-t-have-same-hash.patch
Patch0008: 0008-mmap_cache-Use-stricter-check-for-hash-keys.patch
Patch0501: 0501-FEDORA-Switch-the-default-ccache-location.patch Patch0501: 0501-FEDORA-Switch-the-default-ccache-location.patch
### Dependencies ### ### Dependencies ###
@ -677,6 +686,12 @@ fi
%postun -n libsss_idmap -p /sbin/ldconfig %postun -n libsss_idmap -p /sbin/ldconfig
%changelog %changelog
* Fri Aug 23 2013 Jakub Hrozek <jhrozek@redhat.com> - 1.11.0-0.4.beta2
- Resolves: #967012 - [abrt] sssd-1.9.5-1.fc18: sss_mmap_cache_gr_invalidate_gid:
Process /usr/libexec/sssd/sssd_nss was killed by
signal 11 (SIGSEGV)
- Resolves: #996214 - sssd proxy_child segfault
* Sun Aug 04 2013 Fedora Release Engineering <rel-eng@lists.fedoraproject.org> - 1.11.0-0.3.beta2 * Sun Aug 04 2013 Fedora Release Engineering <rel-eng@lists.fedoraproject.org> - 1.11.0-0.3.beta2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_20_Mass_Rebuild - Rebuilt for https://fedoraproject.org/wiki/Fedora_20_Mass_Rebuild