> On Apr 24, 2025, at 19:56, Kees Cook <[email protected]> wrote:
>
>
>
> On April 24, 2025 1:44:23 PM PDT, Qing Zhao <[email protected]> wrote:
>>
>>
>>> On Apr 24, 2025, at 15:43, Bill Wendling <[email protected]> wrote:
>>>
>>> On Thu, Apr 24, 2025 at 8:15 AM Qing Zhao <[email protected]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Kees reported a segmentation failure when he used the patch to compiler
>>>> kernel,
>>>> and the reduced the testing case is something like the following:
>>>>
>>>> struct f {
>>>> void *g __attribute__((__counted_by__(h)));
>>>> long h;
>>>> };
>>>>
>>>> extern struct f *my_alloc (int);
>>>>
>>>> int i(void) {
>>>> struct f *iov = my_alloc (10);
>>>> int *j = (int *)iov->g;
>>>> return __builtin_dynamic_object_size(iov->g, 0);
>>>> }
>>>>
>>>> Basically, the problem is relating to the pointee type of the pointer
>>>> array being “void”,
>>>> As a result, the element size of the array is not available in the IR.
>>>> Therefore segmentation
>>>> fault when calculating the size of the whole object.
>>>>
>>>> Although it’s easy to fix this segmentation failure, I am not quite sure
>>>> what’s the best
>>>> solution to this issue:
>>>>
>>>> 1. Reject such usage of “counted_by” in the very beginning by reporting
>>>> warning to the
>>>> User, and delete the counted_by attribute from the field.
>>>>
>>>> Or:
>>>>
>>>> 2. Accept such usage, but issue warnings when calculating the object_size
>>>> in Middle-end.
>>>>
>>>> Personally, I prefer the above 1 since I think that when the pointee type
>>>> is void, we don’t know
>>>> The type of the element of the pointer array, there is no way to decide
>>>> the size of the pointer array.
>>>>
>>>> So, the counted_by information is not useful for the
>>>> __builtin_dynamic_object_size.
>>>>
>>>> But I am not sure whether the counted_by still can be used for bound
>>>> sanitizer?
>>>>
>>>> Thanks for suggestions and help.
>>>>
>>> Clang supports __sized_by that can handle a 'void *', where it defaults to
>>> 'u8'.
>
> I would like to be able to use counted_by (and not sized_by) so that users of
> the annotation don't need to have to change the marking just because it's
> "void *". Everything else operates on "void *" as if it were u8 ...
>
> Regardless, ignoring "void *", the rest of my initial testing (of both GCC
> and Clang) is positive. The test cases are all behaving as expected! Yay! :)
> I will try to construct some more goofy stuff to find more corner cases.
Thanks a lot for the help!
>
> And at some future point we may want to think about
> -fsanitize=pointer-overflow using this information too, to catch arithmetic
> and increments past the bounds...
>
> struct foo {
> u8 *buf __counted_by(len);
> int len;
> } str;
> u8 *walk;
> str->buf = malloc(10);
> str->len = 10;
>
> walk = str->buf + 12; // trip!
> for (walk = str->buf; ; walk++) // trip after 10 loops
> ;
>
Add this to my todo list. -:)
thanks.
Qing
>
> -Kees
>
> --
> Kees Cook
