> On Apr 24, 2025, at 14:31, Kees Cook <k...@kernel.org> wrote:
>
> On Thu, Apr 24, 2025 at 06:06:03PM +0000, Qing Zhao wrote:
>>
>>
>>> On Apr 24, 2025, at 13:07, Kees Cook <k...@kernel.org> wrote:
>>>
>>> On Thu, Apr 24, 2025 at 04:36:14PM +0000, Qing Zhao wrote:
>>>>
>>>>> On Apr 24, 2025, at 11:59, Martin Uecker <uec...@tugraz.at> wrote:
>>>>>
>>>>> Am Donnerstag, dem 24.04.2025 um 15:15 +0000 schrieb Qing Zhao:
>>>>>> 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.
>>>>>
>>>>> GNU C allows pointer arithmetic and sizeof on void pointers and
>>>>> that treats void as having size 1. So you could also allow counted_by
>>>>> and assume as size 1 for void.
>>>>>
>>>>> https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html
>>>>
>>>> Okay, thanks for the info.
>>>> So,
>>>> 1. should we issue warnings when doing this?
>>>
>>> Please don't, Linux would very much like to track these allocation sizes
>>> still. Performing pointer arithmetic and bounds checking (via __bdos) on
>>> "void *" is wanted (and such a calculation was what tripped the
>>> segfault).
>>
>> My previous question was: -:)
>>
>> When we accept the “void” pointee type and provide
>> __builtin_dynamic_object_size for
>> such pointers (treating it as 1 byte) shall we issue a warning to users to
>> warn them that the void pointee type is
>> Accepted and treated as size 1?
>>
>> Or just silently handle such case as normal?
>
> I think it should be silently handled. Other such "void is 1 byte" cases
> don't warn.
Okay, that makes sense.
Then I will handle it by default without warning.
>
>>>> 2. If the compilation option is explicitly asking for standard C,
>>>> shall we issue warning and delete the counted_by attribute from the
>>>> field?
>>>
>>> I think it needs to stay attached for __bdos. And from the looks of it,
>>> even array access works with 1-byte values too:
>>>
>>> extern void *ptr;
>>> void *foo(int num) {
>>> return &ptr[num];
>>> }
>>>
>>> The assembly output of this shows it's doing byte addition. Clang
>>> doesn't warn about this, but GCC does:
>>>
>>> <source>:5:16: warning: dereferencing 'void *' pointer
>>> 5 | return &ptr[num];
>>> | ^
>>>
>>> So, I think even the bounds sanitizer should handle it, even if a
>>> warning ultimately gets emitted.
>>
>> Okay. I will also handle the void in bounds sanitizer by treating element
>> size as 1 byte.
>
> Great, that should work well for Linux, and is, I think, the least
> surprising result. :)
>
>>
>> My previous question was:
>>
>> Since this is only a GNU extension, I am wondering under the situation that
>> No GNU extension
>> Is allowed, shall we issue warnings and delete the counted_by attribute?
>
> Oh, like, generally? What GCC option disables GNU extensions?
Actually, I am not sure either on what GCC option disables GNU extensions. -:)
Just a natural question. I tried to add -std=c99 or something, but I didn’t
see behavior change with the testing case.
Qing
>
> -Kees
>
> --
> Kees Cook
