> 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