> 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