ehlo, detail description in attached patches.
LS
>From 295dd1e0966df7bf1fc9a51f84a0daa81a279b5e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <[email protected]> Date: Mon, 19 Aug 2013 05:39:28 +0200 Subject: [PATCH 1/2] mmap_cache: Skip records which doesn't have same hash Record in data table has two hashes hash1 and hash2. Hash table refers to the first record with the searched hash key. We do a record chaining in case of hash collision. When we removed record from cache hash_table was automaticaly updated to the next record from chain. But it is not very likely that two following records will have the same hashes (hash1 and hash2). Therefore it can happen, that some hash table keys refers to records, which both hashes has different values like hash key. Such record can be removed from memory cache, but there will be reference in hash table with different key, which could not be removed, but it points to removed data or in the worst case it points in the middle of newly added record. And this was a reason of crash in nss. Resolves: https://fedorahosted.org/sssd/ticket/2018 --- src/responder/nss/nsssrv_mmap_cache.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c index cd5a6436e005b4c7f5622eaff2f259de3bbe5d29..05d5f61e13cb7716969ef29bd72c2713144a6ab7 100644 --- a/src/responder/nss/nsssrv_mmap_cache.c +++ b/src/responder/nss/nsssrv_mmap_cache.c @@ -134,6 +134,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) @@ -151,7 +172,7 @@ 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; + mcc->hash_table[hash] = sss_mc_get_next_slot_with_hash(mcc, rec, hash); } else { slot = cur->next; while (slot != MC_INVALID_VAL) { @@ -160,7 +181,7 @@ static void sss_mc_rm_rec_from_chain(struct sss_mc_ctx *mcc, if (cur == rec) { /* changing a single uint32_t is atomic, so there is no * need to use barriers in this case */ - prev->next = cur->next; + prev->next = sss_mc_get_next_slot_with_hash(mcc, cur, hash); slot = MC_INVALID_VAL; } else { slot = cur->next; -- 1.8.3.1
>From 40a0179d7692c48d277a1daa39499ae8d4065926 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <[email protected]> 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 | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c index 05d5f61e13cb7716969ef29bd72c2713144a6ab7..6cb2e429f6dfc8c06899a8367b0bb1d41be5bb03 100644 --- a/src/responder/nss/nsssrv_mmap_cache.c +++ b/src/responder/nss/nsssrv_mmap_cache.c @@ -106,7 +106,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; @@ -163,8 +163,9 @@ 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 + 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. This should never happen, but better * return than trying to access out of bounds memory */ return; } -- 1.8.3.1
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
