On 07/15/2015 06:42 PM, Lukas Slebodnik wrote:
ehlo,
reproducer:
add user and few groups to ldap
call id user
remove one group
authenticate
call id user // removed groups should not appear in output
LS
0001-mmap_cache-Override-functions-for-initgr-mmap-cache.patch
From 146337d5a3a15236b0a8c26e10d233e8cbc3a1d8 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik<lsleb...@redhat.com>
Date: Wed, 15 Jul 2015 17:47:29 +0200
Subject: [PATCH] mmap_cache: "Override" functions for initgr mmap cache
Functions sss_mc_get_strs_offset and sss_mc_get_strs_len provides
data about strings for individual memory caches (passwd, ...)
Their are used in generic responder mmap cache code to find a record
in mmap cache (sss_mc_find_record). Data provided from functions sss_mc_get_*
are used for checking the validity of record. So in case of corrupted record
the whole mmap cache can be invalidated.
Functions sss_mc_get_strs_offset and sss_mc_get_strs_len did not provide
data for initgroups mmap cache and therefore particular record could not be
invalidated.
Resolves:
https://fedorahosted.org/sssd/ticket/2716
---
src/responder/nss/nsssrv_mmap_cache.c | 15 ++++++++++++---
src/sss_client/nss_mc_initgr.c | 16 +++++++++++-----
src/util/mmap_cache.h | 6 +++++-
3 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/src/responder/nss/nsssrv_mmap_cache.c
b/src/responder/nss/nsssrv_mmap_cache.c
index
ebda8ac6fab3dd87f5a1d8e43717bf7a5b5a9878..6251ff93ce7f8bf473fc770f7aa3d2e0642395a3
100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -475,6 +475,9 @@ static errno_t sss_mc_get_strs_offset(struct sss_mc_ctx
*mcc,
case SSS_MC_GROUP:
*_offset = offsetof(struct sss_mc_grp_data, strs);
return EOK;
+ case SSS_MC_INITGROUPS:
+ *_offset = offsetof(struct sss_mc_initgr_data, gids);
+ return EOK;
default:
DEBUG(SSSDBG_FATAL_FAILURE, "Unknown memory cache type.\n");
return EINVAL;
@@ -492,6 +495,9 @@ static errno_t sss_mc_get_strs_len(struct sss_mc_ctx *mcc,
case SSS_MC_GROUP:
*_len = ((struct sss_mc_grp_data *)&rec->data)->strs_len;
return EOK;
+ case SSS_MC_INITGROUPS:
+ *_len = ((struct sss_mc_initgr_data *)&rec->data)->data_len;
+ return EOK;
default:
DEBUG(SSSDBG_FATAL_FAILURE, "Unknown memory cache type.\n");
return EINVAL;
@@ -974,8 +980,8 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx
**_mcc,
return EINVAL;
}
- /* memnum + reserved + array of members + name*/
- data_len = (2 + memnum) * sizeof(uint32_t) + name->len;
+ /* array of members + name */
+ data_len = memnum * sizeof(uint32_t) + name->len;
It is actually not array of members but array of group ids
the user is member of. It would be good to rename the
memnum to num_groups or something similiar.
rec_len = sizeof(struct sss_mc_rec) + sizeof(struct sss_mc_initgr_data)
+ data_len;
if (rec_len > mcc->dt_size) {
@@ -998,10 +1004,13 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx
**_mcc,
name->str, name->len, name->str, name->len);
/* initgroups struct */
+ data->strs_len = name->len;
+ data->data_len = data_len;
+ data->reserved = MC_INVALID_VAL32;
data->members = memnum;
^^^^^^^^^^^^^ ^^^^^^
Same as above.
memcpy(data->gids, membuf, memnum * sizeof(uint32_t));
^^^^^^ ^^^^^^
gids_buf
memcpy(&data->gids[memnum], name->str, name->len);
- data->name = MC_PTR_DIFF(&data->gids[memnum], data);
^^^^^^
+ data->strs = data->name = MC_PTR_DIFF(&data->gids[memnum], data);
MC_LOWER_BARRIER(rec);
diff --git a/src/sss_client/nss_mc_initgr.c b/src/sss_client/nss_mc_initgr.c
index
bfb09d6550c310fbab254dc9b3ab7b306b7d3f06..0ccde2130526786e1949287cde1e4c1bc4596670
100644
--- a/src/sss_client/nss_mc_initgr.c
+++ b/src/sss_client/nss_mc_initgr.c
@@ -93,6 +93,7 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t
name_len,
uint32_t hash;
uint32_t slot;
int ret;
+ const size_t data_offset = offsetof(struct sss_mc_initgr_data, gids);
uint8_t *max_addr;
ret = sss_nss_mc_get_ctx("initgroups", &initgr_mc_ctx);
@@ -128,16 +129,21 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name,
size_t name_len,
}
data = (struct sss_mc_initgr_data *)rec->data;
+ rec_name = (char *)data + data->name;
/* Integrity check
- * - array with gids must be within data_table
- * - string must be within data_table */
- if ((uint8_t *)data->gids > max_addr
- || (uint8_t *)data + data->name + name_len > max_addr) {
+ * - name_len cannot be longer than all strings or data
+ * - data->name cannot point outside strings
+ * - all data must be within data_table
+ * - name must be within data_table */
+ if (name_len > data->data_len
+ || name_len > data->strs_len
+ || (data->name + name_len) > (data_offset + data->data_len)
+ || (uint8_t *)data->gids + data->data_len > max_addr
+ || (uint8_t *)rec_name + name_len > max_addr) {
ret = ENOENT;
goto done;
}
- rec_name = (char *)data + data->name;
if (strcmp(name, rec_name) == 0) {
break;
}
diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h
index
438e28a3d217041278fc1bb60aa553d098516035..6cd1e46218fc2a814b34200f0837ee7fff382378
100644
--- a/src/util/mmap_cache.h
+++ b/src/util/mmap_cache.h
@@ -79,7 +79,7 @@ typedef uint32_t rel_ptr_t;
#define SSS_MC_MAJOR_VNO 1
-#define SSS_MC_MINOR_VNO 0
+#define SSS_MC_MINOR_VNO 1
#define SSS_MC_HEADER_UNINIT 0 /* after ftruncate or before reset */
#define SSS_MC_HEADER_ALIVE 1 /* current and in use */
@@ -139,6 +139,10 @@ struct sss_mc_grp_data {
struct sss_mc_initgr_data {
rel_ptr_t name; /* ptr to name string, rel. to struct base addr */
+ rel_ptr_t strs; /* ptr to concatenation of all strings */
^^^^^^^^^^^^^^
This pointer is not needed. The name pointer is enough.
+ uint32_t strs_len; /* length of strs */
^^^^^^^^
This should be called name_len.
+ uint32_t data_len; /* all initgroups data len */
+ uint32_t reserved;
uint32_t members; /* number of members in groups */
^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Again not members, number of gids.
uint32_t gids[0]; /* array of all groups
* string with name is stored after gids */
-- 2.4.3
If you do not want to squash the renaming into the same patch, feel free
to attach it as separate patch. I am fine either way. Sorry I should
have catch this in the first iteration of the initgr memcache patches.
Michal
--
Senior Principal Intern
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
