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.

-Kees

-- 
Kees Cook

Reply via email to