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.
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
;
-Kees
--
Kees Cook
