summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAdrian Johnson <ajohnson@redneon.com>2021-08-15 16:13:15 +0930
committerAdrian Johnson <ajohnson@redneon.com>2021-08-24 07:26:35 +0930
commit068e9b2eb4d2b50e84c8e0debf38e8ee0b955307 (patch)
tree0ba055f5ad1310b6c7a1b0845b1209b176799a55
parent0d809e8051439a4a88a4a97a89e2715d92554a25 (diff)
Fix malloc overflow check warning
To fix this warning: ../src/cairo-malloc-private.h:83:32: warning: comparison is always false due to limited range of data type [-Wtype-limits] 83 | ((size) != 0 && (size_t) (a) >= SIZE_MAX / (size_t) (size) ? NULL : \ | ^~ Create two new macros to do the overflow checks: _cairo_addl_size_t_overflow and _cairo_mul_size_t_overflow. Implement them using compiler builtins where available. Update cairo-malloc-private to use these fuctions and add an overflow test to test the functions.
-rw-r--r--meson.build1
-rw-r--r--src/cairo-compiler-private.h54
-rw-r--r--src/cairo-malloc-private.h56
-rw-r--r--test/meson.build1
-rw-r--r--test/overflow.c214
5 files changed, 312 insertions, 14 deletions
diff --git a/meson.build b/meson.build
index f10a0067b..7a0506712 100644
--- a/meson.build
+++ b/meson.build
@@ -151,6 +151,7 @@ check_headers = [
['fenv.h', {'check-funcs': ['feenableexcept', 'fedisableexcept', 'feclearexcept']}],
['xlocale.h'],
['sys/ioctl.h'],
+ ['intsafe.h'],
]
check_types = [
diff --git a/src/cairo-compiler-private.h b/src/cairo-compiler-private.h
index ffad1cf15..502478a5c 100644
--- a/src/cairo-compiler-private.h
+++ b/src/cairo-compiler-private.h
@@ -42,6 +42,9 @@
#include "config.h"
+#include <stddef.h> /* size_t */
+#include <stdint.h> /* SIZE_MAX */
+
/* Size in bytes of buffer to use off the stack per functions.
* Mostly used by text functions. For larger allocations, they'll
* malloc(). */
@@ -244,4 +247,55 @@
#define inline __inline__
#endif
+/* size_t add/multiply with overflow check.
+ *
+ * These _cairo_fallback_*_size_t_overflow() functions are always defined
+ * to allow them to be tested in the test suite. They are used
+ * if no compiler builtin is available.
+ */
+static cairo_always_inline cairo_bool_t
+_cairo_fallback_add_size_t_overflow(size_t a, size_t b, size_t *c)
+{
+ if (b > SIZE_MAX - a)
+ return 1;
+
+ *c = a + b;
+ return 0;
+}
+
+static cairo_always_inline cairo_bool_t
+_cairo_fallback_mul_size_t_overflow(size_t a, size_t b, size_t *c)
+{
+ if (b != 0 && a > SIZE_MAX / b)
+ return 1;
+
+ *c = a * b;
+ return 0;
+}
+
+/* Clang defines __GNUC__ so check clang builtins before gcc.
+ * MSVC does not support feature macros so hide the __has_builtin inside the #if __clang__ block
+ */
+#ifdef __clang__
+#if defined(__has_builtin) && __has_builtin(__builtin_add_overflow)
+#define _cairo_add_size_t_overflow(a, b, c) __builtin_add_overflow((size_t)(a), (size_t)(b), (size_t*)(c))
+#define _cairo_mul_size_t_overflow(a, b, c) __builtin_mul_overflow((size_t)(a), (size_t)(b), (size_t*)(c))
+#endif
+#elif __GNUC__ >= 8 || (__GNUC__ >= 5 && (INTPTR_MAX == INT64_MAX))
+/* Overflow builtins are available in gcc 5 but the 32-bit version is broken on gcc < 8.
+ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82274
+ */
+#define _cairo_add_size_t_overflow(a, b, c) __builtin_add_overflow((size_t)(a), (size_t)(b), (size_t*)(c))
+#define _cairo_mul_size_t_overflow(a, b, c) __builtin_mul_overflow((size_t)(a), (size_t)(b), (size_t*)(c))
+#elif defined(_MSC_VER) && defined(HAVE_INTSAFE_H)
+#include <intsafe.h>
+#define _cairo_add_size_t_overflow(a,b,c) (SizeTAdd((size_t)(a), (size_t)(b), (size_t*)(c)) != S_OK)
+#define _cairo_mul_size_t_overflow(a,b,c) (SizeTMult((size_t)(a), (size_t)(b), (size_t*)(c)) != S_OK)
+#endif
+
+#ifndef _cairo_add_size_t_overflow
+#define _cairo_add_size_t_overflow _cairo_fallback_add_size_t_overflow
+#define _cairo_mul_size_t_overflow _cairo_fallback_mul_size_t_overflow
+#endif
+
#endif
diff --git a/src/cairo-malloc-private.h b/src/cairo-malloc-private.h
index 40314e1ce..0de52a561 100644
--- a/src/cairo-malloc-private.h
+++ b/src/cairo-malloc-private.h
@@ -79,9 +79,15 @@
* case of malloc() failure or overflow.
**/
-#define _cairo_malloc_ab(a, size) \
- ((size) != 0 && (size_t) (a) >= SIZE_MAX / (size_t) (size) ? NULL : \
- _cairo_malloc((size_t) (a) * (size_t) (size)))
+static cairo_always_inline void *
+_cairo_malloc_ab(size_t a, size_t size)
+{
+ size_t c;
+ if (_cairo_mul_size_t_overflow (a, size, &c))
+ return NULL;
+
+ return _cairo_malloc(c);
+}
/**
* _cairo_realloc_ab:
@@ -101,9 +107,15 @@
* of memory * is left untouched).
**/
-#define _cairo_realloc_ab(ptr, a, size) \
- ((size) != 0 && (size_t) (a) >= SIZE_MAX / (size_t) (size) ? NULL : \
- realloc(ptr, (size_t) (a) * (size_t) (size)))
+static cairo_always_inline void *
+_cairo_realloc_ab(void *ptr, size_t a, size_t size)
+{
+ size_t c;
+ if (_cairo_mul_size_t_overflow (a, size, &c))
+ return NULL;
+
+ return realloc(ptr, c);
+}
/**
* _cairo_malloc_abc:
@@ -122,10 +134,18 @@
* case of malloc() failure or overflow.
**/
-#define _cairo_malloc_abc(a, b, size) \
- ((b) != 0 && (size_t) (a) >= SIZE_MAX / (size_t) (b) ? NULL : \
- (size) != 0 && (size_t) ((a)*(b)) >= SIZE_MAX / (size_t) (size) ? NULL : \
- _cairo_malloc((size_t) (a) * (size_t) (b) * (size_t) (size)))
+static cairo_always_inline void *
+_cairo_malloc_abc(size_t a, size_t b, size_t size)
+{
+ size_t c, d;
+ if (_cairo_mul_size_t_overflow (a, b, &c))
+ return NULL;
+
+ if (_cairo_mul_size_t_overflow (c, size, &d))
+ return NULL;
+
+ return _cairo_malloc(d);
+}
/**
* _cairo_malloc_ab_plus_c:
@@ -141,9 +161,17 @@
* case of malloc() failure or overflow.
**/
-#define _cairo_malloc_ab_plus_c(a, size, c) \
- ((size) != 0 && (size_t) (a) >= SIZE_MAX / (size_t) (size) ? NULL : \
- (size_t) (c) >= SIZE_MAX - (size_t) (a) * (size_t) (size) ? NULL : \
- _cairo_malloc((size_t) (a) * (size_t) (size) + (size_t) (c)))
+static cairo_always_inline void *
+_cairo_malloc_ab_plus_c(size_t a, size_t size, size_t c)
+{
+ size_t d, e;
+ if (_cairo_mul_size_t_overflow (a, size, &d))
+ return NULL;
+
+ if (_cairo_add_size_t_overflow (d, c, &e))
+ return NULL;
+
+ return _cairo_malloc(e);
+}
#endif /* CAIRO_MALLOC_PRIVATE_H */
diff --git a/test/meson.build b/test/meson.build
index 0647d6fc3..73bb0d2d9 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -236,6 +236,7 @@ test_sources = [
'operator-source.c',
'operator-www.c',
'outline-tolerance.c',
+ 'overflow.c',
'over-above-source.c',
'over-around-source.c',
'over-below-source.c',
diff --git a/test/overflow.c b/test/overflow.c
new file mode 100644
index 000000000..85229c73a
--- /dev/null
+++ b/test/overflow.c
@@ -0,0 +1,214 @@
+/*
+ * Copyright © 2021 Adrian Johnson
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy,
+ * modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Author: Adrian Johnson <ajohnson@redneon.com>
+ */
+
+
+#include "cairo-test.h"
+
+#include <cairo.h>
+
+/* Uncomment to enable faulty test data in order to force a
+ * failure. This allows the error logging path to be tested.
+ */
+/* #define FORCE_FAILURE 1 */
+
+struct test_data {
+ uint64_t a;
+ uint64_t b;
+ uint64_t result;
+ cairo_bool_t overflow;
+};
+
+#if SIZEOF_SIZE_T == 4
+static const struct test_data add_32bit_test_data[] = {
+ { 0x00000000, 0x00000000, 0x00000000, 0 },
+ { 0x00000001, 0x00000000, 0x00000001, 0 },
+ { 0x00000000, 0x00000001, 0x00000001, 0 },
+ { 0xffffffff, 0x00000001, 0x00000000, 1 },
+ { 0x00000001, 0xffffffff, 0x00000000, 1 },
+ { 0xfffffffe, 0x00000001, 0xffffffff, 0 },
+ { 0x00000001, 0xfffffffe, 0xffffffff, 0 },
+ { 0x12345678, 0x98765432, 0xaaaaaaaa, 0 },
+ { 0x80000000, 0x80000000, 0x00000000, 1 },
+
+#if FORCE_FAILURE
+ { 0x00000001, 0x00000002, 0x00000004, 1 },
+#endif
+};
+
+static const struct test_data mul_32bit_test_data[] = {
+ { 0x00000000, 0x00000000, 0x00000000, 0 },
+ { 0x0000ffff, 0x0000ffff, 0xfffe0001, 0 },
+ { 0x00010000, 0x00010000, 0x00000000, 1 },
+ { 0x00000002, 0x80000000, 0x00000000, 1 },
+ { 0x80000000, 0x00000002, 0x00000000, 1 },
+ { 0xffffffff, 0x00000001, 0xffffffff, 0 },
+};
+#endif
+
+#if SIZEOF_SIZE_T == 8
+static const struct test_data add_64bit_test_data[] = {
+ { 0x0000000000000000, 0x0000000000000000, 0x0000000000000000, 0 },
+ { 0x0000000000000001, 0x0000000000000000, 0x0000000000000001, 0 },
+ { 0x0000000000000000, 0x0000000000000001, 0x0000000000000001, 0 },
+ { 0xffffffffffffffff, 0x0000000000000001, 0x0000000000000000, 1 },
+ { 0x0000000000000001, 0xffffffffffffffff, 0x0000000000000000, 1 },
+ { 0x0000000000000001, 0xfffffffffffffffe, 0xffffffffffffffff, 0 },
+ { 0xfffffffffffffffe, 0x0000000000000001, 0xffffffffffffffff, 0 },
+ { 0x123456789abcdef0, 0x987654320fedcbba, 0xaaaaaaaaaaaaaaaa, 0 },
+ { 0x8000000000000000, 0x8000000000000000, 0x0000000000000000, 1 },
+
+#if FORCE_FAILURE
+ { 0x0000000000000001, 0x0000000000000002, 0x0000000000000004, 1 },
+#endif
+};
+
+static const struct test_data mul_64bit_test_data[] = {
+ { 0x0000000000000000, 0x0000000000000000, 0x0000000000000000, 0 },
+ { 0x00000000ffffffff, 0x00000000ffffffff, 0xfffffffe00000001, 0 },
+ { 0x0000000100000000, 0x0000000100000000, 0x0000000000000000, 1 },
+ { 0x0000000000000002, 0x8000000000000000, 0x0000000000000000, 1 },
+ { 0x8000000000000000, 0x0000000000000002, 0x0000000000000000, 1 },
+ { 0xffffffffffffffff, 0x0000000000000001, 0xffffffffffffffff, 0 },
+};
+#endif
+
+static cairo_bool_t
+check_if_result_fail (cairo_test_context_t *ctx,
+ size_t result,
+ cairo_bool_t overflow,
+ const struct test_data *data,
+ const char *func_name)
+{
+ int hex_digits = SIZEOF_SIZE_T * 2;
+ if (overflow != data->overflow || (!data->overflow && result != data->result)) {
+ cairo_test_log (ctx, "%s a = 0x%0*llx b = 0x%0*llx result = 0x%0*llx overflow = %d\n",
+ func_name,
+ hex_digits,
+ (unsigned long long)data->a,
+ hex_digits,
+ (unsigned long long)data->b,
+ hex_digits,
+ (unsigned long long)result,
+ overflow);
+ if (data->overflow)
+ cairo_test_log (ctx, "EXPECTED overflow = 1\n");
+ else
+ cairo_test_log (ctx, "EXPECTED result = 0x%0*llx overflow = 0\n",
+ hex_digits,
+ (unsigned long long)data->result);
+ return TRUE;
+ }
+ return FALSE;
+}
+
+static cairo_test_status_t
+preamble (cairo_test_context_t *ctx)
+{
+ int i;
+ cairo_bool_t overflow;
+ size_t result;
+
+#if SIZEOF_SIZE_T == 4
+ const struct test_data *add_data = add_32bit_test_data;
+ int num_add_tests = ARRAY_LENGTH(add_32bit_test_data);
+ const struct test_data *mul_data = mul_32bit_test_data;
+ int num_mul_tests = ARRAY_LENGTH(mul_32bit_test_data);
+#elif SIZEOF_SIZE_T == 8
+ const struct test_data *add_data = add_64bit_test_data;
+ int num_add_tests = ARRAY_LENGTH(add_64bit_test_data);
+ const struct test_data *mul_data = mul_64bit_test_data;
+ int num_mul_tests = ARRAY_LENGTH(mul_64bit_test_data);
+#else
+ cairo_test_log (ctx, "sizeof(size_t) = %lld is not supported by this test\n",
+ (unsigned long long)sizeof(size_t));
+ return CAIRO_TEST_UNTESTED;
+#endif
+
+ /* First check the fallback versions of the overflow functions. */
+ for (i = 0; i < num_add_tests; i++) {
+ const struct test_data *data = &add_data[i];
+ overflow = _cairo_fallback_add_size_t_overflow (data->a, data->b, &result);
+ if (check_if_result_fail (ctx,
+ result,
+ overflow,
+ data,
+ "_cairo_fallback_add_size_t_overflow"))
+ {
+ return CAIRO_TEST_FAILURE;
+ }
+ }
+
+ for (i = 0; i < num_mul_tests; i++) {
+ const struct test_data *data = &mul_data[i];
+ overflow = _cairo_fallback_mul_size_t_overflow (data->a, data->b, &result);
+ if (check_if_result_fail (ctx,
+ result,
+ overflow,
+ data,
+ "_cairo_fallback_mul_size_t_overflow"))
+ {
+ return CAIRO_TEST_FAILURE;
+ }
+ }
+ /* Next check the compiler builtins (if available, otherwise the
+ * fallback versions are tested again). This is to ensure the fallback version
+ * produces idential results to the compiler builtins.
+ */
+ for (i = 0; i < num_add_tests; i++) {
+ const struct test_data *data = &add_data[i];
+ overflow = _cairo_add_size_t_overflow (data->a, data->b, &result);
+ if (check_if_result_fail (ctx,
+ result,
+ overflow,
+ data,
+ "_cairo_add_size_t_overflow"))
+ {
+ return CAIRO_TEST_FAILURE;
+ }
+ }
+
+ for (i = 0; i < num_mul_tests; i++) {
+ const struct test_data *data = &mul_data[i];
+ overflow = _cairo_mul_size_t_overflow (data->a, data->b, &result);
+ if (check_if_result_fail (ctx,
+ result,
+ overflow,
+ data,
+ "_cairo_mul_size_t_overflow"))
+ {
+ return CAIRO_TEST_FAILURE;
+ }
+ }
+
+ return CAIRO_TEST_SUCCESS;
+}
+
+CAIRO_TEST (overflow,
+ "Test the _cairo_*_size_t_overflow functions.",
+ "memory", /* keywords */
+ NULL, /* requirements */
+ 0, 0,
+ preamble, NULL)