On 09/16/2013 06:16 PM, Michal Židek wrote:
+static errno_t sss_mc_rm_rec_from_chain(struct sss_mc_ctx *mcc,
                                       struct sss_mc_rec *rec,
                                       uint32_t hash)

I had bad indentation here, otherwise the attached patch is the same as the previous one.

Michal
>From 32e937650491a907b21af8d3cf322c4708a99169 Mon Sep 17 00:00:00 2001
From: Michal Zidek <[email protected]>
Date: Fri, 13 Sep 2013 17:41:28 +0200
Subject: [PATCH] Check slot validity before MC_SLOT_TO_PTR.

resolves:
https://fedorahosted.org/sssd/ticket/2049
---
 src/responder/nss/nsssrv_mmap_cache.c | 84 ++++++++++++++++++++++++++++++-----
 src/sss_client/nss_mc_common.c        |  4 ++
 2 files changed, 77 insertions(+), 11 deletions(-)

diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index 4e45405..1e1edd7 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -191,6 +191,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 */
@@ -205,16 +212,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;
@@ -223,23 +238,26 @@ 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;
+    uint32_t ret;
 
     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;
+        return EOK;
     }
 
     slot = mcc->hash_table[hash];
@@ -247,18 +265,37 @@ 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);
+        ret = sss_mc_get_next_slot_with_hash(mcc, rec, hash,
+                                             &mcc->hash_table[hash]);
+        if (ret != EOK) {
+            return ret;
+        }
     } 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) {
@@ -278,6 +315,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)
@@ -296,6 +335,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;
@@ -303,9 +344,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);
@@ -353,6 +401,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;
         }
@@ -364,6 +419,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 a0a70ab..6764d80 100644
--- a/src/sss_client/nss_mc_common.c
+++ b/src/sss_client/nss_mc_common.c
@@ -190,6 +190,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);
-- 
1.7.11.2

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to