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? In any
case, I think this should be documented whether this is
supposed to work (or not).
>
> > 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.
Martin
>
> -Kees
>
