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