> On May 26, 2023, at 4:40 PM, Kees Cook <keesc...@chromium.org> wrote:
>
> On Thu, May 25, 2023 at 04:14:47PM +0000, Qing Zhao wrote:
>> GCC will pass the number of elements info from the attached attribute to
>> both
>> __builtin_dynamic_object_size and bounds sanitizer to check the out-of-bounds
>> or dynamic object size issues during runtime for flexible array members.
>>
>> This new feature will provide nice protection to flexible array members
>> (which
>> currently are completely ignored by both __builtin_dynamic_object_size and
>> bounds sanitizers).
>
> Testing went pretty well, though I think I found some bdos issues:
>
> - some things that bdos can't know the size of, and correctly returned
> SIZE_MAX in the past, now thinks are 0-sized.
Will check this issue and fix it.
> - while bdos correctly knows the size of an element_count-annotated
> flexible array, it doesn't know the size of the containing object
> (i.e. it returns SIZE_MAX).
This is a known issue I found during the implementation, and filed a bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109557
for it
And turned out that this was expected behavior.
>
> Also, I think I found a precedence issue:
>
> - if both __alloc_size and 'element_count' are in use, the _smallest_
> of the two is what I would expect to be enforced by the sanitizer
> and reported by __bdos. As is, alloc_size appears to be used when
> it is available, regardless of what 'element_count' shows.
Will check on this and fix it.
>
> I've updated my test cases to show it more clearly, but here is the
> before/after:
>
>
> GCC 13 (correctly does not implement "element_count"):
>
> $ ./array-bounds 2>&1 | grep -v ^'#'
> TAP version 13
> 1..12
> ok 1 global.fixed_size_seen_by_bdos
> ok 2 global.fixed_size_enforced_by_sanitizer
> ok 3 global.unknown_size_unknown_to_bdos
> ok 4 global.unknown_size_ignored_by_sanitizer
> ok 5 global.alloc_size_seen_by_bdos
> ok 6 global.alloc_size_enforced_by_sanitizer
> not ok 7 global.element_count_seen_by_bdos
> not ok 8 global.element_count_enforced_by_sanitizer
> not ok 9 global.alloc_size_with_smaller_element_count_seen_by_bdos
> not ok 10 global.alloc_size_with_smaller_element_count_enforced_by_sanitizer
> ok 11 global.alloc_size_with_bigger_element_count_seen_by_bdos
> ok 12 global.alloc_size_with_bigger_element_count_enforced_by_sanitizer
>
>
> ToT GCC + this element_count series:
>
> $ ./array-bounds 2>&1 | grep -v ^'#'
> TAP version 13
> 1..12
> ok 1 global.fixed_size_seen_by_bdos
> ok 2 global.fixed_size_enforced_by_sanitizer
> not ok 3 global.unknown_size_unknown_to_bdos
> not ok 4 global.unknown_size_ignored_by_sanitizer
> ok 5 global.alloc_size_seen_by_bdos
> ok 6 global.alloc_size_enforced_by_sanitizer
> not ok 7 global.element_count_seen_by_bdos
> ok 8 global.element_count_enforced_by_sanitizer
> not ok 9 global.alloc_size_with_smaller_element_count_seen_by_bdos
> not ok 10 global.alloc_size_with_smaller_element_count_enforced_by_sanitizer
> ok 11 global.alloc_size_with_bigger_element_count_seen_by_bdos
> ok 12 global.alloc_size_with_bigger_element_count_enforced_by_sanitizer
>
>
> Test suite is here:
> https://github.com/kees/kernel-tools/blob/trunk/fortify/array-bounds.c
Thanks a lot for the testing, this is really helpful.
Will study and fix all these issues.
Qing
>
> --
> Kees Cook
