>From 22b9cca2d678f9e3b7870faec2ca066211896b71 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzidek@redhat.com>
Date: Tue, 13 Aug 2013 19:27:21 +0200
Subject: [PATCH 2/2] mmap_cache: Of by one error and triple check removal.

Removes off by one error when using macro MC_SIZE_TO_SLOTS and
also reduces the number of checks in client code, where the
slot number validity is checked.
---
 src/responder/nss/nsssrv_mmap_cache.c | 12 ++++++------
 src/sss_client/nss_mc_group.c         | 34 ++++++++++++++++------------------
 src/sss_client/nss_mc_passwd.c        | 34 ++++++++++++++++------------------
 3 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index cd5a643..a7e9049 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 (slot >= MC_SIZE_TO_SLOTS(mcc->dt_size)) {
         return NULL;
     }
 
     while (slot != MC_INVALID_VAL) {
-        if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+        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);
@@ -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 (slot >= MC_SIZE_TO_SLOTS(mcc->dt_size)) {
         ret = ENOENT;
         goto done;
     }
 
     while (slot != MC_INVALID_VAL) {
-        if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+        if (slot >= MC_SIZE_TO_SLOTS(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 (slot >= MC_SIZE_TO_SLOTS(mcc->dt_size)) {
         ret = ENOENT;
         goto done;
     }
 
     while (slot != MC_INVALID_VAL) {
-        if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+        if (slot >= MC_SIZE_TO_SLOTS(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 3764c9b..5491649 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -105,6 +105,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
     int ret;
     size_t strs_offset;
     uint8_t *max_addr;
+    size_t max_slots;
 
     ret = sss_nss_mc_get_ctx("group", &gr_mc_ctx);
     if (ret) {
@@ -117,16 +118,14 @@ 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;
-        }
+    max_slots = MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size);
 
+    /* The iteration should stop at positive match or if
+     * slot == MC_INVALID_VAL (which is bigger than max_slots).
+     * If the iter. ends with other slot value, the mmap cache
+     * is probbably corrupted. */
+    while (slot < max_slots) {
         ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -159,7 +158,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
         slot = rec->next;
     }
 
-    if (slot == MC_INVALID_VAL) {
+    if (slot >= max_slots) {
         ret = ENOENT;
         goto done;
     }
@@ -182,6 +181,7 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
     uint32_t slot;
     int len;
     int ret;
+    size_t max_slots;
 
     ret = sss_nss_mc_get_ctx("group", &gr_mc_ctx);
     if (ret) {
@@ -196,16 +196,14 @@ 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;
-        }
+    max_slots = MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size);
 
+    /* The iteration should stop at positive match or if
+     * slot == MC_INVALID_VAL (which is bigger than max_slots).
+     * If the iter. ends with other slot value, the mmap cache
+     * is probbably corrupted. */
+    while (slot < max_slots) {
         ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -226,7 +224,7 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
         slot = rec->next;
     }
 
-    if (slot == MC_INVALID_VAL) {
+    if (slot >= max_slots) {
         ret = ENOENT;
         goto done;
     }
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index b29d33b..e750a24 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -106,6 +106,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
     int ret;
     size_t strs_offset;
     uint8_t *max_addr;
+    size_t max_slots;
 
     ret = sss_nss_mc_get_ctx("passwd", &pw_mc_ctx);
     if (ret) {
@@ -118,16 +119,14 @@ 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;
-        }
+    max_slots = MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size);
 
+    /* The iteration should stop at positive match or if
+     * slot == MC_INVALID_VAL (which is bigger than max_slots).
+     * If the iter. ends with other slot value, the mmap cache
+     * is probbably corrupted. */
+    while (slot < max_slots) {
         ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -161,7 +160,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
         slot = rec->next;
     }
 
-    if (slot == MC_INVALID_VAL) {
+    if (slot >= max_slots) {
         ret = ENOENT;
         goto done;
     }
@@ -184,6 +183,7 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
     uint32_t slot;
     int len;
     int ret;
+    size_t max_slots;
 
     ret = sss_nss_mc_get_ctx("passwd", &pw_mc_ctx);
     if (ret) {
@@ -198,16 +198,14 @@ 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;
-        }
+    max_slots = MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size);
 
+    /* The iteration should stop at positive match or if
+     * slot == MC_INVALID_VAL (which is bigger than max_slots).
+     * If the iter. ends with other slot value, the mmap cache
+     * is probbably corrupted. */
+    while (slot < max_slots) {
         ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -228,7 +226,7 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
         slot = rec->next;
     }
 
-    if (slot == MC_INVALID_VAL) {
+    if (slot >= max_slots) {
         ret = ENOENT;
         goto done;
     }
-- 
1.7.11.2


