Browse Source

[LibOS] Fix integer overflow checks using __builtin_sub{add}_overflow

Michał Kowalczyk 6 years ago
parent
commit
5435cd8cc5
2 changed files with 31 additions and 23 deletions
  1. 29 21
      LibOS/shim/src/fs/chroot/fs.c
  2. 2 2
      LibOS/shim/src/sys/shim_brk.c

+ 29 - 21
LibOS/shim/src/fs/chroot/fs.c

@@ -711,8 +711,9 @@ static ssize_t map_read (struct shim_handle * hdl, void * buf, size_t count)
         return ret;
         return ret;
     }
     }
 
 
-    if ((off_t) (marker + count) > file->size)
-        count = file->size - marker;
+    size_t bytes_left;
+    if (!__builtin_sub_overflow(file->size, marker, &bytes_left) && bytes_left < count)
+        count = bytes_left;
 
 
     if (count) {
     if (count) {
         memcpy(buf, file->mapbuf + (marker - file->mapoffset), count);
         memcpy(buf, file->mapbuf + (marker - file->mapoffset), count);
@@ -733,8 +734,15 @@ static ssize_t map_write (struct shim_handle * hdl, const void * buf, size_t cou
     struct shim_file_data * data = FILE_HANDLE_DATA(hdl);
     struct shim_file_data * data = FILE_HANDLE_DATA(hdl);
     off_t marker = file->marker;
     off_t marker = file->marker;
 
 
-    if ((off_t) (file->marker + count) > file->size) {
-        file->size = file->marker + count;
+    off_t new_marker;
+    if (__builtin_add_overflow(marker, count, &new_marker)) {
+        // We can't handle this case reasonably.
+        ret = -EFBIG;
+        goto out;
+    }
+
+    if (new_marker > file->size) {
+        file->size = new_marker;
 
 
         PAL_NUM pal_ret = DkStreamWrite(hdl->pal_handle, file->marker, count, (void *) buf, NULL);
         PAL_NUM pal_ret = DkStreamWrite(hdl->pal_handle, file->marker, count, (void *) buf, NULL);
 
 
@@ -757,8 +765,10 @@ static ssize_t map_write (struct shim_handle * hdl, const void * buf, size_t cou
             } while ((off_t) atomic_cmpxchg(&data->size, size, file->size) != size);
             } while ((off_t) atomic_cmpxchg(&data->size, size, file->size) != size);
         }
         }
 
 
-        assert(marker + pal_ret > 0 && (ssize_t) pal_ret > 0);
-        file->marker = marker + pal_ret;
+        if (__builtin_add_overflow(marker, pal_ret, &file->marker)) {
+            // Should never happen. Even if it would, we couldn't recover from this condition.
+            BUG();
+        }
         ret = (ssize_t) pal_ret;
         ret = (ssize_t) pal_ret;
         goto out;
         goto out;
     }
     }
@@ -769,7 +779,7 @@ static ssize_t map_write (struct shim_handle * hdl, const void * buf, size_t cou
 
 
     if (count) {
     if (count) {
         memcpy(file->mapbuf + (marker - file->mapoffset), buf, count);
         memcpy(file->mapbuf + (marker - file->mapoffset), buf, count);
-        file->marker = marker + count;
+        file->marker = new_marker;
     }
     }
 
 
     ret = count;
     ret = count;
@@ -796,7 +806,8 @@ static ssize_t chroot_read (struct shim_handle * hdl, void * buf, size_t count)
 
 
     struct shim_file_handle * file = &hdl->info.file;
     struct shim_file_handle * file = &hdl->info.file;
 
 
-    if (file->type != FILE_TTY && (off_t) (file->marker + count) < 0) {
+    off_t dummy_off_t;
+    if (file->type != FILE_TTY && __builtin_add_overflow(file->marker, count, &dummy_off_t)) {
         ret = -EFBIG;
         ret = -EFBIG;
         goto out;
         goto out;
     }
     }
@@ -814,12 +825,10 @@ static ssize_t chroot_read (struct shim_handle * hdl, void * buf, size_t count)
 
 
     PAL_NUM pal_ret = DkStreamRead(hdl->pal_handle, file->marker, count, buf, NULL, 0);
     PAL_NUM pal_ret = DkStreamRead(hdl->pal_handle, file->marker, count, buf, NULL, 0);
     if (pal_ret > 0) {
     if (pal_ret > 0) {
-        ret = (ssize_t) pal_ret;
-        assert(ret > 0);
-        if (file->type != FILE_TTY) {
-            assert(file->marker + pal_ret > 0);
-            file->marker += pal_ret;
-        }
+        if (__builtin_add_overflow(pal_ret, 0, &ret))
+            BUG();
+        if (file->type != FILE_TTY && __builtin_add_overflow(file->marker, pal_ret, &file->marker))
+            BUG();
     } else {
     } else {
         ret = PAL_NATIVE_ERRNO == PAL_ERROR_ENDOFSTREAM ?  0 : -PAL_ERRNO;
         ret = PAL_NATIVE_ERRNO == PAL_ERROR_ENDOFSTREAM ?  0 : -PAL_ERRNO;
     }
     }
@@ -847,7 +856,8 @@ static ssize_t chroot_write (struct shim_handle * hdl, const void * buf, size_t
 
 
     struct shim_file_handle * file = &hdl->info.file;
     struct shim_file_handle * file = &hdl->info.file;
 
 
-    if (file->type != FILE_TTY && (off_t) (file->marker + count) < 0) {
+    off_t dummy_off_t;
+    if (file->type != FILE_TTY && __builtin_add_overflow(file->marker, count, &dummy_off_t)) {
         ret = -EFBIG;
         ret = -EFBIG;
         goto out;
         goto out;
     }
     }
@@ -865,12 +875,10 @@ static ssize_t chroot_write (struct shim_handle * hdl, const void * buf, size_t
 
 
     PAL_NUM pal_ret = DkStreamWrite(hdl->pal_handle, file->marker, count, (void *) buf, NULL);
     PAL_NUM pal_ret = DkStreamWrite(hdl->pal_handle, file->marker, count, (void *) buf, NULL);
     if (pal_ret > 0) {
     if (pal_ret > 0) {
-        ret = (ssize_t) pal_ret;
-        assert(ret > 0);
-        if (file->type != FILE_TTY) {
-            assert(file->marker + pal_ret > 0);
-            file->marker += pal_ret;
-        }
+        if (__builtin_add_overflow(pal_ret, 0, &ret))
+            BUG();
+        if (file->type != FILE_TTY && __builtin_add_overflow(file->marker, pal_ret, &file->marker))
+            BUG();
     } else {
     } else {
         ret = PAL_NATIVE_ERRNO == PAL_ERROR_ENDOFSTREAM ?  0 : -PAL_ERRNO;
         ret = PAL_NATIVE_ERRNO == PAL_ERROR_ENDOFSTREAM ?  0 : -PAL_ERRNO;
     }
     }

+ 2 - 2
LibOS/shim/src/sys/shim_brk.c

@@ -95,8 +95,8 @@ int init_brk_region (void * brk_region)
                 int ret = DkRandomBitsRead(&rand, sizeof(rand));
                 int ret = DkRandomBitsRead(&rand, sizeof(rand));
                 if (ret < 0)
                 if (ret < 0)
                     return -convert_pal_errno(-ret);
                     return -convert_pal_errno(-ret);
-                rand %= MIN((uint32_t) 0x2000000,
-                            (uint32_t) (PAL_CB(user_address.end) - brk_region - brk_max_size));
+                rand %= MIN((size_t)0x2000000,
+                            (size_t)(PAL_CB(user_address.end) - brk_region - brk_max_size));
                 rand = ALIGN_DOWN(rand);
                 rand = ALIGN_DOWN(rand);
 
 
                 if (brk_region + rand + brk_max_size >= PAL_CB(user_address.end))
                 if (brk_region + rand + brk_max_size >= PAL_CB(user_address.end))