Created attachment 47305 [details] PoC for -fsanitize=bounds When choosing which arrays to instrument, -fsanitize=bounds tries to avoid flexible array members. Traditionally, this has been either [], [0], or [1]-sized arrays (though the latter two are realistically considered deprecated in C). However, the sanitizer appears to be ignoring _all_ trailing arrays in a structure, no matter what their size. Comparing the behavior between GCC and Clang, this is more visible: $ gcc -Wall -g3 -fsanitize=bounds -fsanitize-undefined-trap-on-error -o bounds-gcc bounds.c $ ./bounds-gcc abc flex non_flex non_trailing Illegal instruction (core dumped) $ clang -Wall -g3 -fsanitize=bounds -fsanitize-undefined-trap-on-error -o bounds-clang bounds.c $ ./bounds-clang abc flex non_flex Illegal instruction (core dumped) I would expect the trap during the non_flex structure over-index, as seen in the Clang-built binary.
I thought GCC documented this differently. So this is just a documentation issue. GCC allows even non-1 sized fields to be considered flexible arrays if they are at the end of the struct.
Is there anything to enforce a strict "only consider empty array size as flexible array member" mode? This is an unfortunate weakening of the array bounds checker as there are plenty of structures that have a fixed-size array as the final member.
From what I remember is there are actually arrays in POSIX and other older code bases that are not just one element which needs to be treated this way. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68270#c1 There are others too. I can't find them right now though. >Is there anything to enforce a strict "only consider empty array size as flexible array member" mode? NO. See above of why.
(In reply to Kees Cook from comment #2) > Is there anything to enforce a strict "only consider empty array size as > flexible array member" mode? This is an unfortunate weakening of the array > bounds checker as there are plenty of structures that have a fixed-size > array as the final member. There is -fsanitize=bounds-strict.
(In reply to Andrew Pinski from comment #1) > I thought GCC documented this differently. So this is just a documentation > issue. > GCC allows even non-1 sized fields to be considered flexible arrays if they > are at the end of the struct. GCC might handle it gracefully in many contexts but it's considered a bug (in user code) and warnings like -Wstringop-overflow try to detect and diagnose it, so it's something to avoid. At -O3, current trunk issues: pr92589.c:41:27: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=] 41 | non_flex->data[i] = argv[1][i]; | ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ pr92589.c:14:10: note: destination object declared here 14 | char data[2]; | ^~~~ GCC manual (Zero Length Arrays) documents the [] and [0] forms and discourages the [1] form of trailing arrays. There is no mention of arrays with more elements being treated that way.
(In reply to Jakub Jelinek from comment #4) > (In reply to Kees Cook from comment #2) > > Is there anything to enforce a strict "only consider empty array size as > > flexible array member" mode? This is an unfortunate weakening of the array > > bounds checker as there are plenty of structures that have a fixed-size > > array as the final member. > > There is -fsanitize=bounds-strict. This is too strict: it doesn't allow flexible arrays ([]) either. I'd like something that ignores _only_ flexible arrays and fails on all other trailing arrays beyond their size.
(In reply to Kees Cook from comment #6) > (In reply to Jakub Jelinek from comment #4) > > (In reply to Kees Cook from comment #2) > > > Is there anything to enforce a strict "only consider empty array size as > > > flexible array member" mode? This is an unfortunate weakening of the array > > > bounds checker as there are plenty of structures that have a fixed-size > > > array as the final member. > > > > There is -fsanitize=bounds-strict. > > This is too strict: it doesn't allow flexible arrays ([]) either. I'd like > something that ignores _only_ flexible arrays and fails on all other > trailing arrays beyond their size. Oops, sorry, my PoC was testing the corner cases, not the correct cases. -fsanitize=bounds-strict _does_ work (though I'd rather it disallowed [0], but I'll live): $ gcc -Wall -g3 -fsanitize=bounds-strict -fsanitize-undefined-trap-on-error -o bounds-gcc bounds.c $ ./bounds-gcc abc flex (should always be okay): ok (no trap!) zero (should be okay, treated as flex): ok (no trap!) one (should fail): Illegal instruction (core dumped)
Created attachment 48153 [details] updated PoC