> On Apr 24, 2025, at 19:56, Kees Cook <k...@kernel.org> wrote:
> 
> 
> 
> On April 24, 2025 1:44:23 PM PDT, Qing Zhao <qing.z...@oracle.com> wrote:
>> 
>> 
>>> On Apr 24, 2025, at 15:43, Bill Wendling <isanb...@gmail.com> wrote:
>>> 
>>> On Thu, Apr 24, 2025 at 8:15 AM Qing Zhao <qing.z...@oracle.com> 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


Reply via email to