> 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?
>
>> 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.
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?
Qing
>
> -Kees
>
> --
> Kees Cook
