> On Aug 27, 2024, at 02:17, Martin Uecker <uec...@tugraz.at> wrote:
>
> Am Montag, dem 26.08.2024 um 17:21 -0700 schrieb Kees Cook:
>> On Mon, Aug 26, 2024 at 11:01:08PM +0200, Martin Uecker wrote:
>>> Am Montag, dem 26.08.2024 um 13:30 -0700 schrieb Kees Cook:
>>>> On Mon, Aug 26, 2024 at 07:30:15PM +0000, Qing Zhao wrote:
>>>>> Hi, Martin,
>>>>>
>>>>> Looks like that there is some issue when I tried to use the _Generic for
>>>>> the testing cases, and then I narrowed down to a
>>>>> small testing case that shows the problem without any change to GCC.
>>>>>
>>>>> [opc@qinzhao-ol8u3-x86 gcc]$ cat t1.c
>>>>> struct annotated {
>>>>> char b;
>>>>> int c[];
>>>>> } *array_annotated;
>>>>> extern void * counted_by_ref (int *);
>>>>>
>>>>> int main(int argc, char *argv[])
>>>>> {
>>>>> typeof(counted_by_ref (array_annotated->c)) ret
>>>>> = counted_by_ref (array_annotated->c);
>>>>> _Generic (ret, void* : (void)0, default: *ret = 10);
>>>>>
>>>>> return 0;
>>>>> }
>>>>> [opc@qinzhao-ol8u3-x86 gcc]$ /home/opc/Install/latest/bin/gcc t1.c
>>>>> t1.c: In function ‘main’:
>>>>> t1.c:12:44: warning: dereferencing ‘void *’ pointer
>>>>> 12 | _Generic (ret, void* : (void)0, default: *ret = 10);
>>>>> | ^~~~
>>>>> t1.c:12:49: error: invalid use of void expression
>>>>> 12 | _Generic (ret, void* : (void)0, default: *ret = 10);
>>>>> | ^
>>>>
>>>> I implemented it like this[1] in the Linux kernel. So yours could be:
>>>>
>>>> struct annotated {
>>>> char b;
>>>> int c[] __attribute__((counted_by(b));
>>>> };
>>>> extern struct annotated *array_annotated;
>>>>
>>>> int main(int argc, char *argv[])
>>>> {
>>>> typeof(_Generic(__builtin_get_counted_by(array_annotated->c),
>>>> void *: (size_t *)NULL,
>>>> default: __builtin_get_counted_by(array_annotated->c)))
>>>> ret = __builtin_get_counted_by(array_annotated->c);
>>>> if (ret)
>>>> *ret = 10;
>>>>
>>>> return 0;
>>>> }
>>>>
>>>> It's a bit cumbersome, but it does what's needed.
>>>>
>>>> This is, however, just doing exactly what Bill has suggested: it is
>>>> converting the (void *)NULL into (size_t *)NULL when there is no
>>>> counted_by annotation...
>>>>
>>>> -Kees
>>>>
>>>> [1]
>>>> https://lore.kernel.org/linux-hardening/20240822231324.make.666-k...@kernel.org/
>>>
>>> Interesting. Will __builtin_get_counted_by(array_annotated->c) give
>>> a null pointer (or an invalid pointer) of the correct type if
>>> array_annotated is a null pointer of an annotated struct type?
>>
>> If you mean this part:
>>
>> typeof(P) __obj_ptr = NULL; \
>> /* Just query the counter type for type_max checking. */ \
>> typeof(_Generic(__flex_counter(__obj_ptr->FAM), \
>> void *: (size_t *)NULL, \
>> default: __flex_counter(__obj_ptr->FAM))) \
>> __counter_type_ptr = NULL; \
>>
>> Where __obj_ptr starts as NULL, then yes. (Or at least, yes it does
>> currently with Qing's GCC patch and Bill's Clang patch.)
>
> Does __builtin_get_counted_by not evaluate its argument?
__builtin_get_counted_by currently is implemented as a C reserved words, and
purely implemented in C parser as an C operator.
So, it doesn’t apply complicated evaluations on its argument.
I think that this should provide enough and simple functionality as needed.
If no, please let me know.
> In any
> case, I think this should be documented whether this is
> supposed to work (or not).
Okay.
>
>>
>>> I also wonder a bit about the multiple macro evaluations of the arguments
>>> for P and SIZE.
>>
>> I tried to design it so they aren't used with anything that should
>> have side-effects.
>
> I was more concerned about the cost of macro expansions on
> compile times. I would do:
>
> __auto_type __FOO = (FOO);
>
> for all macro parameters that are evaluated multiple times
> and are expressions which might contain macros themselves.
>
> There is also the issue of evaluation of typeof for variably modified
> types, which might not currently affect the kernel, but this would
> also become safer for such types.
>
>
>> Anyway, if __builtin_get_counted_by returns (size_t *)NULL then I think
>> the _Generic wrapping isn't needed. That would make it easier to use?
>
> It would make it easier for your use case. I wonder though
> whether other people might want to have the compile time error
> when there is no attribute.
Then this will go back to the previous discussion: whether we should go the
approach for the unary operator __counted_by(PTR), then the other builtin
__builtin_has_attribute(PTR, counted_by) should be provided to the user.
For GCC, there is no issue, we already have the __builtin_has_attribute (PTR,
counted_by) available.
But CLANG doesn’t have this builtin currently, need to implement it before the
unary operator __counted_by(PTR).
Let me know your opinion.
Thanks
Qing
>
>
> Martin
>
>>
>> -Kees
