From af3fa359b4b7335008652b9f1eefbd41ac8216d9 Mon Sep 17 00:00:00 2001 From: Martin Sebor Date: Tue, 19 Dec 2017 12:14:57 -0700 Subject: [PATCH] PR middle-end/77608 - missing protection on trivially detectable runtime buffer overflow gcc/ChangeLog: PR middle-end/77608 * builtins.c (compute_objsize): Handle non-constant offsets. gcc/testsuite/ChangeLog: PR middle-end/77608 * gcc.dg/Wstringop-overflow.c: New test. * gcc/testsuite/c-c++-common/Warray-bounds-3.c: Adjust. From-SVN: r255836 --- gcc/ChangeLog | 6 + gcc/builtins.c | 70 ++++++++-- gcc/testsuite/ChangeLog | 7 + gcc/testsuite/c-c++-common/Warray-bounds-3.c | 2 +- gcc/testsuite/gcc.dg/Wstringop-overflow.c | 132 +++++++++++++++++++ 5 files changed, 206 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/Wstringop-overflow.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 5e8e9bf6acc..4581f41d8fb 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2017-12-19 Martin Sebor + + PR middle-end/77608 + * builtins.c (compute_objsize): Handle non-constant offsets. + 2017-12-19 Jakub Jelinek PR tree-optimization/83444 @@ -190,6 +195,7 @@ * tree-ssa-dom.c (record_equivalences_from_phis): Fix handling of degenerates resulting from ignoring an edge. +>>>>>>> .r255835 2017-12-18 Martin Sebor PR middle-end/83373 diff --git a/gcc/builtins.c b/gcc/builtins.c index 4b06f64c630..6bff904d8be 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -3278,8 +3278,12 @@ check_access (tree exp, tree, tree, tree dstwrite, /* Helper to compute the size of the object referenced by the DEST expression which must have pointer type, using Object Size type OSTYPE (only the least significant 2 bits are used). Return - the size of the object if successful or NULL when the size cannot - be determined. */ + an estimate of the size of the object if successful or NULL when + the size cannot be determined. When the referenced object involves + a non-constant offset in some range the returned value represents + the largest size given the smallest non-negative offset in the + range. The function is intended for diagnostics and should not + be used to influence code generation or optimization. */ tree compute_objsize (tree dest, int ostype) @@ -3292,24 +3296,57 @@ compute_objsize (tree dest, int ostype) if (compute_builtin_object_size (dest, ostype, &size)) return build_int_cst (sizetype, size); - /* Unless computing the largest size (for memcpy and other raw memory - functions), try to determine the size of the object from its type. */ - if (!ostype) - return NULL_TREE; - if (TREE_CODE (dest) == SSA_NAME) { gimple *stmt = SSA_NAME_DEF_STMT (dest); if (!is_gimple_assign (stmt)) return NULL_TREE; + dest = gimple_assign_rhs1 (stmt); + tree_code code = gimple_assign_rhs_code (stmt); - if (code != ADDR_EXPR && code != POINTER_PLUS_EXPR) - return NULL_TREE; + if (code == POINTER_PLUS_EXPR) + { + /* compute_builtin_object_size fails for addresses with + non-constant offsets. Try to determine the range of + such an offset here and use it to adjus the constant + size. */ + tree off = gimple_assign_rhs2 (stmt); + if (TREE_CODE (off) == SSA_NAME + && INTEGRAL_TYPE_P (TREE_TYPE (off))) + { + wide_int min, max; + enum value_range_type rng = get_range_info (off, &min, &max); - dest = gimple_assign_rhs1 (stmt); + if (rng == VR_RANGE) + { + if (tree size = compute_objsize (dest, ostype)) + { + wide_int wisiz = wi::to_wide (size); + + /* Ignore negative offsets for now. For others, + use the lower bound as the most optimistic + estimate of the (remaining)size. */ + if (wi::sign_mask (min)) + ; + else if (wi::ltu_p (min, wisiz)) + return wide_int_to_tree (TREE_TYPE (size), + wi::sub (wisiz, min)); + else + return size_zero_node; + } + } + } + } + else if (code != ADDR_EXPR) + return NULL_TREE; } + /* Unless computing the largest size (for memcpy and other raw memory + functions), try to determine the size of the object from its type. */ + if (!ostype) + return NULL_TREE; + if (TREE_CODE (dest) != ADDR_EXPR) return NULL_TREE; @@ -3471,6 +3508,19 @@ expand_builtin_mempcpy (tree exp, rtx target) tree src = CALL_EXPR_ARG (exp, 1); tree len = CALL_EXPR_ARG (exp, 2); + /* Policy does not generally allow using compute_objsize (which + is used internally by check_memop_size) to change code generation + or drive optimization decisions. + + In this instance it is safe because the code we generate has + the same semantics regardless of the return value of + check_memop_sizes. Exactly the same amount of data is copied + and the return value is exactly the same in both cases. + + Furthermore, check_memop_size always uses mode 0 for the call to + compute_objsize, so the imprecise nature of compute_objsize is + avoided. */ + /* Avoid expanding mempcpy into memcpy when the call is determined to overflow the buffer. This also prevents the same overflow from being diagnosed again when expanding memcpy. */ diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 1146e94c0e9..63d671374e3 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2017-12-19 Martin Sebor + + PR middle-end/77608 + * gcc.dg/Wstringop-overflow.c: New test. + * gcc/testsuite/c-c++-common/Warray-bounds-3.c: Adjust. + 2017-12-19 Alexandre Oliva PR debug/83422 @@ -57,6 +63,7 @@ PR ipa/83346 * g++.dg/ipa/pr82801.C: New test. +>>>>>>> .r255835 2017-12-18 Martin Sebor PR middle-end/83373 diff --git a/gcc/testsuite/c-c++-common/Warray-bounds-3.c b/gcc/testsuite/c-c++-common/Warray-bounds-3.c index 85c8f6a6d02..73103f43d01 100644 --- a/gcc/testsuite/c-c++-common/Warray-bounds-3.c +++ b/gcc/testsuite/c-c++-common/Warray-bounds-3.c @@ -1,7 +1,7 @@ /* Exercise that -Warray-bounds is issued for out-of-bounds offsets in calls to built-in functions. { dg-do compile } - { dg-options "-O2 -Warray-bounds -ftrack-macro-expansion=0" } */ + { dg-options "-O2 -Wno-stringop-overflow -Warray-bounds -ftrack-macro-expansion=0" } */ #include "../gcc.dg/range.h" diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow.c b/gcc/testsuite/gcc.dg/Wstringop-overflow.c new file mode 100644 index 00000000000..b5bd40ed958 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow.c @@ -0,0 +1,132 @@ +/* PR middle-end/77608 - missing protection on trivially detectable runtime + buffer overflow + { dg-do compile } + { dg-options "-O2 -Wstringop-overflow -ftrack-macro-expansion=0" } */ + +#define SIZE_MAX __SIZE_MAX__ +#define DIFF_MAX __PTRDIFF_MAX__ +#define DIFF_MIN (-DIFF_MAX - 1) + +typedef __SIZE_TYPE__ size_t; + +extern void* memcpy (void*, const void*, size_t); +extern char* strcpy (char*, const char*); +extern char* strncpy (char*, const char*, size_t); + +void sink (void*); + +extern size_t unsigned_value (void) +{ + extern volatile size_t unsigned_value_source; + return unsigned_value_source; +} + +size_t unsigned_range (size_t min, size_t max) +{ + size_t val = unsigned_value (); + return val < min || max < val ? min : val; +} + +#define UR(min, max) unsigned_range (min, max) + + +char a7[7]; + +struct MemArray { char a9[9]; char a1[1]; }; + +void test_memcpy_array (const void *s) +{ +#define T(d, s, n) (memcpy ((d), (s), (n)), sink (d)) + + T (a7 + UR (0, 1), s, 7); + T (a7 + UR (0, 7), s, 7); + T (a7 + UR (0, 8), s, 7); + T (a7 + UR (0, DIFF_MAX), s, 7); + T (a7 + UR (0, SIZE_MAX), s, 7); + + T (a7 + UR (1, 2), s, 7); /* { dg-warning "writing 7 bytes into a region of size 6" } */ + T (a7 + UR (2, 3), s, 7); /* { dg-warning "writing 7 bytes into a region of size 5" } */ + T (a7 + UR (6, 9), s, 7); /* { dg-warning "writing 7 bytes into a region of size 1" } */ + T (a7 + UR (7, 9), s, 7); /* { dg-warning "writing 7 bytes into a region of size 0" } */ + T (a7 + UR (8, 9), s, 7); /* { dg-warning "writing 7 bytes into a region of size 0" } */ + + T (a7 + UR (9, 10), s, 7); /* { dg-warning "writing 7 bytes into a region of size 0" } */ + T (a7 + UR (DIFF_MAX, DIFF_MAX + (size_t)1), s, 7); /* { dg-warning "writing 7 bytes into a region of size 0" } */ + T (a7 + UR (DIFF_MAX, SIZE_MAX), s, 7); /* { dg-warning "writing 7 bytes into a region of size 0" } */ + + /* This is valid. */ + char *d = a7 + 7; + T (d + UR (-8, -7), s, 7); +} + +/* Verify the absence of warnings for memcpy writing beyond object + boundaries. */ + +void test_memcpy_memarray (struct MemArray *p, const void *s) +{ +#undef T +#define T(d, s, n) (memcpy ((d), (s), (n)), sink (d)) + + /* The following are valid. */ + T (p->a9 + UR (0, 1), s, 9); + T (p->a9 + UR (0, 7), s, 9); + T (p->a9 + UR (0, 8), s, 9); + T (p->a9 + UR (0, DIFF_MAX), s, 9); + T (p->a9 + UR (0, SIZE_MAX), s, 9); + + /* The following are invalid. Unfortunately, there is apparently enough + code out there that abuses memcpy to write past the end of one member + and into the members that follow so the following are not diagnosed + by design. It sure would be nice not to have to cater to hacks like + these... */ + T (p->a9 + UR (1, 2), s, 9); + T (p->a9 + UR (1, 2), s, 123); +} + + +void test_strcpy_array (void) +{ +#undef T +#define T(d, s) (strcpy ((d), (s)), sink (d)) + + T (a7 + UR (0, 1), "012345"); + T (a7 + UR (0, 7), "012345"); + T (a7 + UR (0, 8), "012345"); + T (a7 + UR (0, DIFF_MAX), "012345"); + T (a7 + UR (0, SIZE_MAX), "012345"); + + T (a7 + UR (1, 2), "012345"); /* { dg-warning "writing 7 bytes into a region of size 6" } */ + T (a7 + UR (2, 3), "012345"); /* { dg-warning "writing 7 bytes into a region of size 5" } */ + T (a7 + UR (6, 9), "012345"); /* { dg-warning "writing 7 bytes into a region of size 1" } */ + T (a7 + UR (7, 9), "012345"); /* { dg-warning "writing 7 bytes into a region of size 0" } */ + T (a7 + UR (8, 9), "012345"); /* { dg-warning "writing 7 bytes into a region of size 0" } */ + + T (a7 + UR (9, 10), "012345"); /* { dg-warning "writing 7 bytes into a region of size 0" } */ + T (a7 + UR (DIFF_MAX, DIFF_MAX + (size_t)1), "012345"); /* { dg-warning "writing 7 bytes into a region of size 0" } */ + T (a7 + UR (DIFF_MAX, SIZE_MAX), "012345"); /* { dg-warning "writing 7 bytes into a region of size 0" } */ + + char *d = a7 + 7; + + T (d + UR (-8, -7), "012345"); +} + +void test_strncpy_memarray (struct MemArray *p, const void *s) +{ +#undef T +#define T(d, s, n) (strncpy ((d), (s), (n)), sink (d)) + + T (p->a9 + UR (0, 1), s, 9); + T (p->a9 + UR (0, 7), s, 9); + T (p->a9 + UR (0, 8), s, 9); + T (p->a9 + UR (0, DIFF_MAX), s, 9); + T (p->a9 + UR (0, SIZE_MAX), s, 9); + + T (p->a9 + UR (1, 2), s, 9); /* { dg-warning "writing 9 bytes into a region of size 8" } */ + T (p->a9 + UR (2, 3), s, 9); /* { dg-warning "writing 9 bytes into a region of size 7" } */ + T (p->a9 + UR (6, 9), s, 9); /* { dg-warning "writing 9 bytes into a region of size 3" } */ + T (p->a9 + UR (9, 10), s, 9); /* { dg-warning "writing 9 bytes into a region of size 0" } */ + T (p->a9 + UR (10, 11), s, 9); /* { dg-warning "writing 9 bytes into a region of size 0" } */ + + T (p->a9 + UR (DIFF_MAX, DIFF_MAX + (size_t)1), s, 1); /* { dg-warning "writing 1 byte into a region of size 0" } */ + T (p->a9 + UR (DIFF_MAX, SIZE_MAX), s, 3); /* { dg-warning "writing 3 bytes into a region of size 0" } */ +} -- 2.30.2