Changeset 94795812 in mainline

Timestamp:

2012-11-05T21:52:35Z (12 years ago)

Author:

Jakub Jermar <jakub@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

2d53cfc, 33c2952, efdfebc

Parents:

0941e9ae

Message:

Check for dangerous unsigned integer overflows in check_area_conflicts().

• Rename overflows_add() to overflows_into_positive().
• Introduce overflows() for simple sum overflow tests.

Location:

kernel/generic

Files:

• include/macros.h (modified) (1 diff)
• src/mm/as.c (modified) (8 diffs)

kernel/generic/include/macros.h

@@ -130 +130 @@
             | ((((uint64_t) (up)) & UINT32_C(0xffffffff)) << 32))
 
-/* Test for result wrap-around into positive numbers. */
-#define overflows_add(a, b) \
-        (((a) + (b) < (a)) && ((a) + (b)))
+/* Test for sum overflow. */
+#define overflows(a, b) \
+        ((a) + (b) < (a))
+
+/* Test for sum overflow into positive numbers. */
+#define overflows_into_positive(a, b)   \
+        (overflows((a), (b)) && ((a) + (b) > 0))
 
 /** Pseudorandom generator

kernel/generic/src/mm/as.c

@@ -299 +299 @@
         ASSERT((addr % PAGE_SIZE) == 0);
         ASSERT(mutex_locked(&as->lock));
-        ASSERT(!overflows_add(addr, P2SZ(count)));
+
+        /*
+         * If the addition of the supposed area address and size overflows,
+         * report conflict.
+         */
+        if (overflows_into_positive(addr, P2SZ(count)))
+                return false;
         
         /*
@@ -331 +337 @@
                         mutex_lock(&area->lock);
 
-                        /* If at least one of the two areas are protected
+                        /*
+                         * If at least one of the two areas are protected
                          * by the AS_AREA_GUARD flag then we must be sure
                          * that they are separated by at least one unmapped
@@ -339 +346 @@
                             (area->flags & AS_AREA_GUARD)) ? 1 : 0;
                         
+                        /*
+                         * The area comes from the left neighbour node, which
+                         * means that there already are some areas in the leaf
+                         * node, which in turn means that adding gp is safe and
+                         * will not cause an integer overflow.
+                         */
                         if (overlaps(addr, P2SZ(count), area->base,
                             P2SZ(area->pages + gp))) {
@@ -354 +367 @@
                 
                 if (area != avoid) {
+                        int gp;
+
                         mutex_lock(&area->lock);
 
-                        int const gp = (guarded ||
-                            (area->flags & AS_AREA_GUARD)) ? 1 : 0;
+                        gp = (guarded || (area->flags & AS_AREA_GUARD)) ? 1 : 0;
+                        if (gp && overflows(addr, P2SZ(count))) {
+                                /*
+                                 * Guard page not needed if the supposed area
+                                 * is adjacent to the end of the address space.
+                                 * We already know that the following test is
+                                 * going to fail...
+                                 */
+                                gp--;
+                        }
                         
                         if (overlaps(addr, P2SZ(count + gp), area->base,
@@ -373 +396 @@
         for (i = 0; i < leaf->keys; i++) {
                 area = (as_area_t *) leaf->value[i];
+                int agp;
+                int gp;
                 
                 if (area == avoid)
@@ -379 +404 @@
                 mutex_lock(&area->lock);
 
-                int const gp = (guarded ||
-                    (area->flags & AS_AREA_GUARD)) ? 1 : 0;
+                gp = (guarded || (area->flags & AS_AREA_GUARD)) ? 1 : 0;
+                agp = gp;
+
+                /*
+                 * Sanitize the two possible unsigned integer overflows.
+                 */
+                if (gp && overflows(addr, P2SZ(count)))
+                        gp--;
+                if (agp && overflows(area->base, P2SZ(area->pages)))
+                        agp--;
 
                 if (overlaps(addr, P2SZ(count + gp), area->base,
-                    P2SZ(area->pages + gp))) {
+                    P2SZ(area->pages + agp))) {
                         mutex_unlock(&area->lock);
                         return false;
@@ -533 +566 @@
         }
 
-        if (overflows_add(*base, size))
+        if (overflows_into_positive(*base, size))
                 return NULL;
 
@@ -816 +849 @@
                  */
 
-                if (overflows_add(address, P2SZ(pages)))
+                if (overflows_into_positive(address, P2SZ(pages)))
                         return EINVAL;