> On Apr 24, 2025, at 13:07, Kees Cook <[email protected]> wrote:
>
> On Thu, Apr 24, 2025 at 04:36:14PM +0000, Qing Zhao wrote:
>>
>>> On Apr 24, 2025, at 11:59, Martin Uecker <[email protected]> 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).
>
>> 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.
I tried to come up with a testing case for array sanitizer on void pointers as
following:
#include <stdlib.h>
struct annotated {
int b;
void *c __attribute__ ((counted_by (b)));
} *array_annotated;
void __attribute__((__noinline__)) setup (int annotated_count)
{
array_annotated
= (struct annotated *)malloc (sizeof (struct annotated));
array_annotated->c = malloc (sizeof (char) * annotated_count);
array_annotated->b = annotated_count;
return;
}
void __attribute__((__noinline__)) test (int annotated_index)
{
array_annotated->c[annotated_index] = 2;
}
int main(int argc, char *argv[])
{
setup (10);
test (10);
return 0;
}
When I compile it, I always got the following error:
: In function ‘test’:
:25:21: warning: dereferencing ‘void *’ pointer
:25:39: error: invalid use of void expression
Looks like that the void pointer cannot be accessed as an array?
thanks.
Qing
>
> -Kees
>
> --
> Kees Cook