__builtin_object_size fails for POINTER_PLUS expressions involving non-constant offsets into objects of known size, causing GCC to fail to detect (and add instrumentation to prevent) buffer overflow in some basic cases such as the following:
void f (unsigned i) { char d [3]; memcpy (d + i, "abcdef", 5); } Since the size of the destination object is known, the call to memcpy is guaranteed to write past the end of it regardless of the value of the offset. The attached patch enhances __builtin_object_size to handle this case by returning the size of the whole object as the maximum and the size of the object minus T_MAX for the type of the offset T as the minimum. The patch also adds handling of ranges even though only very few cases benefit from it because the VRP pass runs after the object size pass. The one case that does appear to profit is when the value of the offset is constrained by its type, as in char a [1000]; unsigned g (unsigned char i) { char *p = a + i; return __builtin_object_size (p, 2); } Here get_range_info () lets __builtin_object_size determine that the minimum number of bytes between (a + i) and (a + sizeof s) is sizeof a - UCHAR_MAX. The patch results in 64 more checking calls in a Binutils build than before. Martin PS What would be a good way to arrange for the VRP pass to run before the object size pass so that the latter can benefit more from range information? As an experiment I added another instance of the VRP pass before the object size pass in passes.def and that worked, but I suspect that running VRP a third time isn't optimal.
PR middle-end/77608 - missing protection on trivially detectable runtime buffer overflow gcc/testsuite/ChangeLog: PR middle-end/77608 * gcc.dg/builtin-object-size-18.c: New test. gcc/ChangeLog: PR middle-end/77608 * tree-object-size.c (plus_stmt_object_size): Handle non-constant offsets. diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-18.c b/gcc/testsuite/gcc.dg/builtin-object-size-18.c new file mode 100644 index 0000000..e8e2269 --- /dev/null +++ b/gcc/testsuite/gcc.dg/builtin-object-size-18.c @@ -0,0 +1,76 @@ +/* PR 77608 - missing protection on trivially detectable runtime buffer + overflow */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +#define concat(a, b) a ## b +#define CAT(a, b) concat (a, b) + +/* Create a symbol name unique to each tes and object size type. */ +#define SYM(type) CAT (CAT (CAT (failure_on_line_, __LINE__), _type_), type) + +/* References to the following undefined symbol which is unique for each + test case are expected to be eliminated. */ +#define TEST_FAILURE(type) \ + do { \ + extern void SYM (type)(void); \ + SYM (type)(); \ + } while (0) + +#define bos(obj, type) __builtin_object_size (obj, type) +#define size(obj, n) ((size_t)n == X ? sizeof *obj : (size_t)n) + +/* For convenience. Substitute for 'sizeof object' in test cases where + the size can vary from target to target. */ +#define X (size_t)0xdeadbeef + +#define test(expect, type, obj) \ + do { \ + if (bos (obj, type) != size (obj, expect)) \ + TEST_FAILURE (type); \ + } while (0) + +/* Verify that each of the expressions __builtin_object_size(OBJ, n) + is folded into Rn. */ +#define fold(r0, r1, r2, r3, obj) \ + do { \ + test (r0, 0, (obj)); \ + test (r1, 1, (obj)); \ + test (r2, 2, (obj)); \ + test (r3, 3, (obj)); \ + } while (0) + +/* Verify that __builtin_object_size(xBUF + OFF, n) is folded into Rn + for the specified OFF and each of the Rn values. */ +#define FOLD(off, r0, r1, r2, r3) \ + do { \ + fold (r0, r1, r2, r3, gbuf + off); \ + sink (gbuf); \ + fold (r0, r1, r2, r3, lbuf + off); \ + sink (lbuf); \ + fold (r0, r1, r2, r3, abuf + off); \ + sink (abuf); \ + } while (0) + +#define UCHAR_MAX (__SCHAR_MAX__ * 2 + 1) +#define BUFSIZE (UCHAR_MAX * 2) + +typedef __SIZE_TYPE__ size_t; + +void sink (void*, ...); + +char gbuf[BUFSIZE]; + +void test_pointer_plus (char ci, short si, int i, long li) +{ + char lbuf[BUFSIZE]; + + char *abuf = __builtin_malloc (BUFSIZE); + + FOLD (ci, BUFSIZE, BUFSIZE, BUFSIZE - UCHAR_MAX, BUFSIZE - UCHAR_MAX); + FOLD (si, BUFSIZE, BUFSIZE, 0, 0); + FOLD ( i, BUFSIZE, BUFSIZE, 0, 0); + FOLD (li, BUFSIZE, BUFSIZE, 0, 0); +} + +/* { dg-final { scan-tree-dump-not "failue_on_line" "optimized" } } */ diff --git a/gcc/tree-object-size.c b/gcc/tree-object-size.c index 1317ad7..9e84156 100644 --- a/gcc/tree-object-size.c +++ b/gcc/tree-object-size.c @@ -837,21 +839,49 @@ plus_stmt_object_size (struct object_size_info *osi, tree var, gimple *stmt) if (object_sizes[object_size_type][varno] == unknown[object_size_type]) return false; + bool estimated = false; + + /* If the OFFSET isn't constant try to determine its range and use + either the lower or the upper bound of the range in its place. */ + if (TREE_CODE (op1) == SSA_NAME) + { + wide_int min, max; + enum value_range_type range_type = get_range_info (op1, &min, &max); + if (range_type == VR_RANGE) + { + if (object_size_type < 2) + op1 = build_int_cst (TREE_TYPE (op1), wi::fits_uhwi_p (min) + ? min.to_uhwi () : min.to_shwi ()); + else + op1 = build_int_cst (TREE_TYPE (op1), wi::fits_uhwi_p (max) + ? max.to_uhwi () : max.to_shwi ()); + } + } + + /* If the OFFSET is not constant, assume it's either zero for the maximum + object size or SIZE_MAX for the minimum size of zero. */ + if (TREE_CODE (op1) != INTEGER_CST || !tree_fits_uhwi_p (op1)) + { + op1 = (object_size_type < 2 + ? integer_zero_node : build_all_ones_cst (size_type_node)); + estimated = true; + } + /* Handle PTR + OFFSET here. */ - if (TREE_CODE (op1) == INTEGER_CST - && (TREE_CODE (op0) == SSA_NAME - || TREE_CODE (op0) == ADDR_EXPR)) + if (TREE_CODE (op0) == SSA_NAME + || TREE_CODE (op0) == ADDR_EXPR) { - if (! tree_fits_uhwi_p (op1)) - bytes = unknown[object_size_type]; - else if (TREE_CODE (op0) == SSA_NAME) - return merge_object_sizes (osi, var, op0, tree_to_uhwi (op1)); + unsigned HOST_WIDE_INT off = tree_to_uhwi (op1); + + if (TREE_CODE (op0) == SSA_NAME) + return merge_object_sizes (osi, var, op0, off); else { - unsigned HOST_WIDE_INT off = tree_to_uhwi (op1); - - /* op0 will be ADDR_EXPR here. */ + /* op0 will be ADDR_EXPR here. */ addr_object_size (osi, op0, object_size_type, &bytes); + if (estimated && off == HOST_WIDE_INT_M1U) + off = bytes; + if (bytes == unknown[object_size_type]) ; else if (off > offset_limit)