An update on the kernel building with my version 4 patch.
Kees reported two FE issues with the current version 4 patch:
1. The operator “typeof” cannot return correct type for a->array;
2. The operator “&” cannot return correct address for a->array;
I fixed both in my local repository.
With these additional fix. Kernel with counted-by annotation can be built
successfully.
And then, Kees reported one behavioral issue with the current counted-by:
When the counted-by value is below zero, my current patch
A. Didn’t report any warning for it.
B. Accepted the negative value as a wrapped size.
i.e. for:
struct foo {
signed char size;
unsigned char array[] __counted_by(size);
} *a;
...
a->size = -3;
report(__builtin_dynamic_object_size(p->array, 1));
this reports 253, rather than 0.
And the array-bounds sanitizer doesn’t catch negative index bounds neither.
a->size = -3;
report(a->array[1]); // does not trap
So, my questions are:
How should we handle the negative counted-by value?
My approach is:
I think that this is a user error, the compiler need to Issue warning during
runtime about this user error.
Since I have one remaining patch that has not been finished yet:
6 Emit warnings when the user breaks the requirments for the new counted_by
attribute
compilation time: -Wcounted-by
run time: -fsanitizer=counted-by
* The initialization to the size field should be done before the first
reference to the FAM field.
* the array has at least # of elements specified by the size field all the
time during the program.
* the value of counted-by should not be negative.
Let me know your comment and suggestions.
Thanks
Qing
> On Jan 25, 2024, at 3:11 PM, Qing Zhao <qing.z...@oracle.com> wrote:
>
> Thanks a lot for the testing.
>
> Yes, I can repeat the issue with the following small example:
>
> #include <stdlib.h>
> #include <stddef.h>
> #include <stdio.h>
>
> #define MAX(a, b) ((a) > (b) ? (a) : (b))
>
> struct untracked {
> int size;
> int array[] __attribute__((counted_by (size)));
> } *a;
> struct untracked * alloc_buf (int index)
> {
> struct untracked *p;
> p = (struct untracked *) malloc (MAX (sizeof (struct untracked),
> (offsetof (struct untracked, array[0])
> + (index) * sizeof (int))));
> p->size = index;
> return p;
> }
>
> int main()
> {
> a = alloc_buf(10);
> printf ("same_type is %d\n",
> (__builtin_types_compatible_p(typeof (a->array), typeof (&(a->array)[0]))));
> return 0;
> }
>
>
> /home/opc/Install/latest-d/bin/gcc -O2 btcp.c
> same_type is 1
>
> Looks like that the “typeof” operator need to be handled specially in C FE
> for the new internal function .ACCESS_WITH_SIZE.
>
> (I have specially handle the operator “offsetof” in C FE already).
>
> Will fix this issue.
>
> Thanks.
>
> Qing
>
>> On Jan 24, 2024, at 7:51 PM, Kees Cook <keesc...@chromium.org> wrote:
>>
>> On Wed, Jan 24, 2024 at 12:29:51AM +0000, Qing Zhao wrote:
>>> This is the 4th version of the patch.
>>
>> Thanks very much for this!
>>
>> I tripped over an unexpected behavioral change that the Linux kernel
>> depends on:
>>
>> __builtin_types_compatible_p() no longer treats an array marked with
>> counted_by as different from that array's decayed pointer. Specifically,
>> the kernel uses these macros:
>>
>>
>> /*
>> * Force a compilation error if condition is true, but also produce a
>> * result (of value 0 and type int), so the expression can be used
>> * e.g. in a structure initializer (or where-ever else comma expressions
>> * aren't permitted).
>> */
>> #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
>>
>> #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
>>
>> /* &a[0] degrades to a pointer: a different type from an array */
>> #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
>>
>>
>> This gets used in various places to make sure we're dealing with an
>> array for a macro:
>>
>> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
>> __must_be_array(arr))
>>
>>
>> So this builds:
>>
>> struct untracked {
>> int size;
>> int array[];
>> } *a;
>>
>> __must_be_array(a->array)
>> => 0 (as expected)
>> __builtin_types_compatible_p(typeof(a->array), typeof(&(a->array)[0]))
>> => 0 (as expected, array vs decayed array pointer)
>>
>>
>> But if counted_by is added, we get a build failure:
>>
>> struct tracked {
>> int size;
>> int array[] __counted_by(size);
>> } *b;
>>
>> __must_be_array(b->array)
>> => build failure (not expected)
>> __builtin_types_compatible_p(typeof(b->array), typeof(&(b->array)[0]))
>> => 1 (not expected, both pointers?)
>>
>>
>>
>>
>> --
>> Kees Cook
>
