Re: [V1][PATCH 0/3] New attribute "element_count" to annotate bounds for C99 FAM(PR108896)

[Kees Cook <keescook@chromium.org>]

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.
- 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).

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.

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

-- 
Kees Cook