[cairo-commit] 4 commits - meson.build src/cairo-compiler-private.h src/cairo-malloc-private.h test/Makefile.sources test/meson.build test/mime-unique-id.c test/overflow.c

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Fri Aug 27 23:44:10 UTC 2021


 meson.build                  |    1 
 src/cairo-compiler-private.h |   54 ++++++++++
 src/cairo-malloc-private.h   |   56 ++++++++---
 test/Makefile.sources        |    1 
 test/meson.build             |    1 
 test/overflow.c              |  214 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 313 insertions(+), 14 deletions(-)

New commits:
commit 5e76dd7a5c0585515eeef5099f12b273c94d3b0f
Merge: 9bb1cbf72 a8c8363e3
Author: Adrian Johnson <ajohnson at redneon.com>
Date:   Fri Aug 27 23:44:08 2021 +0000

    Merge branch 'fix-comparison-warning' into 'master'
    
    Fix comparison is always false in malloc overflow check
    
    See merge request cairo/cairo!236

commit a8c8363e35ceb78db05be29f6839145a8c97bdd4
Author: Adrian Johnson <ajohnson at redneon.com>
Date:   Thu Aug 26 21:05:55 2021 +0930

    Add overflow to Makefile.sources

diff --git a/test/Makefile.sources b/test/Makefile.sources
index 084e53343..d962a57fb 100644
--- a/test/Makefile.sources
+++ b/test/Makefile.sources
@@ -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				\
commit 068e9b2eb4d2b50e84c8e0debf38e8ee0b955307
Author: Adrian Johnson <ajohnson at redneon.com>
Date:   Sun Aug 15 16:13:15 2021 +0930

    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.

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 at 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)
commit 0d809e8051439a4a88a4a97a89e2715d92554a25
Author: Adrian Johnson <ajohnson at redneon.com>
Date:   Sun Aug 15 12:54:44 2021 +0930

    Fix file mode of mime_unqiue-id.c

diff --git a/test/mime-unique-id.c b/test/mime-unique-id.c
old mode 100755
new mode 100644


More information about the cairo-commit mailing list