> 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).
> 
>> 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

Reply via email to