prdb_check: fix out of bounds array access in continuation entries
authorMichael Meffie <mmeffie@sinenomine.net>
Wed, 18 Feb 2015 02:54:46 +0000 (21:54 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Wed, 18 Nov 2015 04:48:12 +0000 (23:48 -0500)
A continuation entry (struct contentry) contains 39 id elements, however
a regular entry (struct prentry) contains only 10 id elements.
Attempting to access more than 10 elements of a regular entry is
undefined behavior.

Use a stuct contentry when when processing continuation entries in
prdb_check.  This is done to safely traverse the id arrays of the
continuation entries.  Use the new pr_PrintContEntry to print
continuation entries.

The undefined behavior manfests as a segmentation violation in
WalkNextChain() when built with GCC 4.8 with optimization enabled.

Change-Id: I7613345ee6b7b232c5a0645f4f302c3eac0cdc15
Reviewed-on: http://gerrit.openafs.org/11742
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

src/ptserver/db_verify.c

index 4803076..a8e4916 100644 (file)
@@ -260,6 +260,13 @@ PrintEntryError(struct misc_data *misc, afs_int32 ea, struct prentry *e, int ind
     return 0;
 }
 
+int
+PrintContError(struct misc_data *misc, afs_int32 ea, struct contentry *c, int indent)
+{
+    pr_PrintContEntry(stderr, /*net order */ 0, ea, c, indent);
+    return 0;
+}
+
 afs_int32
 WalkHashTable(afs_int32 hashtable[],   /* hash table to walk */
              int hashType,             /* hash function to use */
@@ -385,7 +392,7 @@ WalkNextChain(char map[],           /* one byte per db entry */
     afs_int32 head;
     int bit;
     afs_int32 code;
-    struct prentry c;          /* continuation entry */
+    struct contentry c;                /* continuation entry */
     afs_int32 na;              /* next thread */
     int ni;
     afs_int32 eid = 0;
@@ -493,7 +500,7 @@ WalkNextChain(char map[],           /* one byte per db entry */
                return PRDBBAD;
            if (na != sghead) {
                fprintf(stderr, "last block: \n");
-               if (PrintEntryError(misc, na, &c, 4))
+               if (PrintContError(misc, na, &c, 4))
                    return PRDBBAD;
            }
            return 0;
@@ -507,7 +514,7 @@ WalkNextChain(char map[],           /* one byte per db entry */
            fprintf(stderr, "Continuation entry reused\n");
            if (PrintEntryError(misc, ea, e, 2))
                return PRDBBAD;
-           if (PrintEntryError(misc, na, &c, 4))
+           if (PrintContError(misc, na, &c, 4))
                return PRDBBAD;
            noErrors = 0;
            break;
@@ -517,7 +524,7 @@ WalkNextChain(char map[],           /* one byte per db entry */
            fprintf(stderr, "Continuation id mismatch\n");
            if (PrintEntryError(misc, ea, e, 2))
                return PRDBBAD;
-           if (PrintEntryError(misc, na, &c, 4))
+           if (PrintContError(misc, na, &c, 4))
                return PRDBBAD;
            noErrors = 0;
            continue;
@@ -541,7 +548,7 @@ WalkNextChain(char map[],           /* one byte per db entry */
                            "User can't be member of supergroup list\n");
                    if (PrintEntryError(misc, ea, e, 2))
                        return PRDBBAD;
-                   if (PrintEntryError(misc, na, &c, 4))
+                   if (PrintContError(misc, na, &c, 4))
                        return PRDBBAD;
                    noErrors = 0;
                }
@@ -568,7 +575,7 @@ WalkNextChain(char map[],           /* one byte per db entry */
                return PRDBBAD;
            if (na != head) {
                fprintf(stderr, "last block: \n");
-               if (PrintEntryError(misc, na, &c, 4))
+               if (PrintContError(misc, na, &c, 4))
                    return PRDBBAD;
            }
            return 0;
@@ -584,7 +591,7 @@ WalkNextChain(char map[],           /* one byte per db entry */
                fprintf(stderr, "walking free list");
            else if (PrintEntryError(misc, ea, e, 2))
                return PRDBBAD;
-           if (PrintEntryError(misc, na, &c, 4))
+           if (PrintContError(misc, na, &c, 4))
                return PRDBBAD;
            noErrors = 0;
            break;
@@ -596,7 +603,7 @@ WalkNextChain(char map[],           /* one byte per db entry */
                fprintf(stderr, "walking free list");
            else if (PrintEntryError(misc, ea, e, 2))
                return PRDBBAD;
-           if (PrintEntryError(misc, na, &c, 4))
+           if (PrintContError(misc, na, &c, 4))
                return PRDBBAD;
            noErrors = 0;
            continue;
@@ -626,7 +633,7 @@ WalkNextChain(char map[],           /* one byte per db entry */
                                "User can't be member of user in membership list\n");
                        if (PrintEntryError(misc, ea, e, 2))
                            return PRDBBAD;
-                       if (PrintEntryError(misc, na, &c, 4))
+                       if (PrintContError(misc, na, &c, 4))
                            return PRDBBAD;
                        noErrors = 0;
                    }
@@ -636,7 +643,7 @@ WalkNextChain(char map[],           /* one byte per db entry */
                                "Bad user/group dicotomy in membership list\n");
                        if (PrintEntryError(misc, ea, e, 2))
                            return PRDBBAD;
-                       if (PrintEntryError(misc, na, &c, 4))
+                       if (PrintContError(misc, na, &c, 4))
                            return PRDBBAD;
                        noErrors = 0;
                    }
@@ -1197,7 +1204,7 @@ DumpRecreate(char map[], struct misc_data *misc)
                }
                na = ntohl(e.next);
                while (na) {
-                   struct prentry c;
+                   struct contentry c;
                    code = pr_Read(na, (char *)&c, sizeof(c));
                    if (code)
                        return code;