From 48323e596876a5b4a6c493aede44b3db7df23164 Mon Sep 17 00:00:00 2001 From: Michal Zidek Date: Fri, 13 Sep 2013 17:41:28 +0200 Subject: Check slot validity before MC_SLOT_TO_PTR. resolves: https://fedorahosted.org/sssd/ticket/2049 --- src/responder/nss/nsssrv_mmap_cache.c | 90 +++++++++++++++++++++++++++++------ src/sss_client/nss_mc_common.c | 4 ++ 2 files changed, 79 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c index 7645fa49e..249d52ff5 100644 --- a/src/responder/nss/nsssrv_mmap_cache.c +++ b/src/responder/nss/nsssrv_mmap_cache.c @@ -182,6 +182,13 @@ static void sss_mc_add_rec_to_chain(struct sss_mc_ctx *mcc, } do { + 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); + return; + } + cur = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec); if (cur == rec) { /* rec already stored in hash chain */ @@ -196,16 +203,24 @@ 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 +static inline errno_t sss_mc_get_next_slot_with_hash(struct sss_mc_ctx *mcc, struct sss_mc_rec *start_rec, - uint32_t hash) + uint32_t hash, + uint32_t *_slot) { struct sss_mc_rec *rec; uint32_t slot; slot = start_rec->next; while (slot != MC_INVALID_VAL) { + 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); + return EINVAL; + } + rec = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec); if (rec->hash1 == hash || rec->hash2 == hash) { break; @@ -214,23 +229,27 @@ sss_mc_get_next_slot_with_hash(struct sss_mc_ctx *mcc, slot = rec->next; } - return slot; + *_slot = slot; + + return EOK; } -static void sss_mc_rm_rec_from_chain(struct sss_mc_ctx *mcc, - struct sss_mc_rec *rec, - uint32_t hash) +static errno_t sss_mc_rm_rec_from_chain(struct sss_mc_ctx *mcc, + struct sss_mc_rec *rec, + uint32_t hash) { struct sss_mc_rec *prev = NULL; struct sss_mc_rec *cur = NULL; uint32_t slot; if (hash > MC_HT_ELEMS(mcc->ht_size)) { - /* It can happen if rec->hash1 and rec->hash2 was the same. - * or it is invalid hash. It is better to return - * than trying to access out of bounds memory - */ - return; + if (hash == MC_INVALID_VAL) { + /* This can happen if rec->hash1 and rec->hash2 was the same. */ + return EOK; + } + + /* The hash is invalid. */ + return EINVAL; } slot = mcc->hash_table[hash]; @@ -238,18 +257,34 @@ static void sss_mc_rm_rec_from_chain(struct sss_mc_ctx *mcc, /* record has already been removed. It may happen if rec->hash1 and * rec->has2 are the same. (It is not very likely). */ - return; + return EOK; } + + 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); + return EINVAL; + } + cur = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec); if (cur == rec) { /* 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); + return sss_mc_get_next_slot_with_hash(mcc, rec, hash, + &mcc->hash_table[hash]); } else { slot = cur->next; while (slot != MC_INVALID_VAL) { + 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); + return EINVAL; + } + prev = cur; cur = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec); if (cur == rec) { @@ -269,6 +304,8 @@ static void sss_mc_rm_rec_from_chain(struct sss_mc_ctx *mcc, } } } + + return EOK; } static void sss_mc_free_slots(struct sss_mc_ctx *mcc, struct sss_mc_rec *rec) @@ -287,6 +324,8 @@ static void sss_mc_free_slots(struct sss_mc_ctx *mcc, struct sss_mc_rec *rec) static void sss_mc_invalidate_rec(struct sss_mc_ctx *mcc, struct sss_mc_rec *rec) { + errno_t ret; + if (rec->b1 == MC_INVALID_VAL) { /* record already invalid */ return; @@ -294,9 +333,16 @@ static void sss_mc_invalidate_rec(struct sss_mc_ctx *mcc, /* Remove from hash chains */ /* hash chain 1 */ - sss_mc_rm_rec_from_chain(mcc, rec, rec->hash1); + ret = sss_mc_rm_rec_from_chain(mcc, rec, rec->hash1); + if (ret != EOK) { + return; + } + /* hash chain 2 */ - sss_mc_rm_rec_from_chain(mcc, rec, rec->hash2); + ret = sss_mc_rm_rec_from_chain(mcc, rec, rec->hash2); + if (ret != EOK) { + return; + } /* Clear from free_table */ sss_mc_free_slots(mcc, rec); @@ -345,6 +391,13 @@ static bool sss_mc_is_valid_rec(struct sss_mc_ctx *mcc, struct sss_mc_rec *rec) self = NULL; slot = mcc->hash_table[rec->hash1]; while (slot != MC_INVALID_VAL32 && self != rec) { + 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); + return false; + } + self = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec); slot = self->next; } @@ -356,6 +409,13 @@ static bool sss_mc_is_valid_rec(struct sss_mc_ctx *mcc, struct sss_mc_rec *rec) self = NULL; slot = mcc->hash_table[rec->hash2]; while (slot != MC_INVALID_VAL32 && self != rec) { + 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); + return false; + } + self = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec); slot = self->next; } diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c index bb2412a1b..3fa28cc53 100644 --- a/src/sss_client/nss_mc_common.c +++ b/src/sss_client/nss_mc_common.c @@ -187,6 +187,10 @@ errno_t sss_nss_mc_get_record(struct sss_cli_mc_ctx *ctx, int count; int ret; + if (!MC_SLOT_WITHIN_BOUNDS(slot, ctx->dt_size)) { + return EINVAL; + } + /* try max 5 times */ for (count = 5; count > 0; count--) { rec = MC_SLOT_TO_PTR(ctx->data_table, slot, struct sss_mc_rec); -- cgit