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.)

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

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?

-Kees

-- 
Kees Cook

Reply via email to