shared: Add defensive measures to array handling

- Make sure that SIZE_MAX boundaries are never crossed
- Clear pointer address in struct during array_free_array
- Do nothing if array_pop is called with empty array

Also added test case for pop behavior and extended tests with
more checks.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Link: https://github.com/kmod-project/kmod/pull/68
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>

diff --git a/shared/array.c b/shared/array.c
index 8f0dd0d41f32162436bc9e2cf6691502f00f930e..ee776a3a6443dd721489faad33a6c4bdff7d3cdd 100644 (file)
--- a/shared/array.c
+++ b/shared/array.c
@@ -5,6 +5,7 @@
 
 #include <assert.h>
 #include <errno.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -15,7 +16,11 @@
 
 static int array_realloc(struct array *array, size_t new_total)
 {
-       void *tmp = realloc(array->array, sizeof(void *) * new_total);
+       void *tmp;
+
+       if (SIZE_MAX / sizeof(void *) < new_total)
+               return -ENOMEM;
+       tmp = realloc(array->array, sizeof(void *) * new_total);
        if (tmp == NULL)
                return -ENOMEM;
        array->array = tmp;
@@ -37,7 +42,11 @@ int array_append(struct array *array, const void *element)
        size_t idx;
 
        if (array->count + 1 >= array->total) {
-               int r = array_realloc(array, array->total + array->step);
+               int r;
+
+               if (SIZE_MAX - array->total < array->step)
+                       return -ENOMEM;
+               r = array_realloc(array, array->total + array->step);
                if (r < 0)
                        return r;
        }
@@ -58,6 +67,8 @@ int array_append_unique(struct array *array, const void *element)
 }
 
 void array_pop(struct array *array) {
+       if (array->count == 0)
+               return;
        array->count--;
        if (array->count + array->step < array->total) {
                int r = array_realloc(array, array->total - array->step);
@@ -68,6 +79,7 @@ void array_pop(struct array *array) {
 
 void array_free_array(struct array *array) {
        free(array->array);
+       array->array = NULL;
        array->count = 0;
        array->total = 0;
 }

diff --git a/testsuite/test-array.c b/testsuite/test-array.c
index 61350f86ea448794053a1f27a647308dbe878225..838dcec9c70bab6adb615fbf8fc083cba492cd10 100644 (file)
--- a/testsuite/test-array.c
+++ b/testsuite/test-array.c
@@ -130,13 +130,16 @@ static int test_array_remove_at(const struct test *t)
        assert_return(array.count == 2, EXIT_FAILURE);
        assert_return(array.array[0] == c1, EXIT_FAILURE);
        assert_return(array.array[1] == c2, EXIT_FAILURE);
+       assert_return(array.total == 4, EXIT_FAILURE);
 
        array_remove_at(&array, 0);
        assert_return(array.count == 1, EXIT_FAILURE);
        assert_return(array.array[0] == c2, EXIT_FAILURE);
+       assert_return(array.total == 2, EXIT_FAILURE);
 
        array_remove_at(&array, 0);
        assert_return(array.count == 0, EXIT_FAILURE);
+       assert_return(array.total == 2, EXIT_FAILURE);
 
        array_append(&array, c1);
        array_append(&array, c2);
@@ -146,6 +149,7 @@ static int test_array_remove_at(const struct test *t)
        assert_return(array.count == 2, EXIT_FAILURE);
        assert_return(array.array[0] == c1, EXIT_FAILURE);
        assert_return(array.array[1] == c3, EXIT_FAILURE);
+       assert_return(array.total == 4, EXIT_FAILURE);
 
        array_free_array(&array);
 
@@ -178,6 +182,10 @@ static int test_array_pop(const struct test *t)
 
        assert_return(array.count == 0, EXIT_FAILURE);
 
+       array_pop(&array);
+
+       assert_return(array.count == 0, EXIT_FAILURE);
+
        array_free_array(&array);
 
        return 0;