Re: [PATCH v5 6/6] kernel.h: handle pointers to arrays better in container_of()

[Ian Abbott]

On 25/05/17 19:35, Kees Cook wrote:
> On Thu, May 25, 2017 at 5:03 AM, Ian Abbott <abbotti@xxxxxxxxx> wrote:
> > If the first parameter of container_of() is a pointer to a
> > non-const-qualified array type (and the third parameter names a
> > non-const-qualified array member), the local variable __mptr will be
> > defined with a const-qualified array type.  In ISO C, these types are
> > incompatible.  They work as expected in GNU C, but some versions will
> > issue warnings.  For example, GCC 4.9 produces the warning
> > "initialization from incompatible pointer type".
> >
> > Here is an example of where the problem occurs:
> >
> > -------------------------------------------------------
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >
> > MODULE_LICENSE("GPL");
> >
> > struct st {
> >         int a;
> >         char b[16];
> > };
> >
> > static int __init example_init(void) {
> >         struct st t = { .a = 101, .b = "hello" };
> >         char (*p)[16] = &t.b;
> >         struct st *x = container_of(p, struct st, b);
> >         printk(KERN_DEBUG "%p %p\n", (void *)&t, (void *)x);
> >         return 0;
> > }
> >
> > static void __exit example_exit(void) {
> > }
> >
> > module_init(example_init);
> > module_exit(example_exit);
> > -------------------------------------------------------
> >
> > Building the module with gcc-4.9 results in these warnings (where '{m}'
> > is the module source and '{k}' is the kernel source):
> >
> > -------------------------------------------------------
> > In file included from {m}/example.c:1:0:
> > {m}/example.c: In function âexample_initâ:
> > {k}/include/linux/kernel.h:854:48: warning: initialization from
> > incompatible pointer type
> >   const typeof( ((type *)0)->member ) *__mptr = (ptr); \
> >                                                 ^
> > {m}/example.c:14:17: note: in expansion of macro âcontainer_ofâ
> >   struct st *x = container_of(p, struct st, b);
> >                  ^
> > {k}/include/linux/kernel.h:854:48: warning: (near initialization for
> > âxâ)
> >   const typeof( ((type *)0)->member ) *__mptr = (ptr); \
> >                                                 ^
> > {m}/example.c:14:17: note: in expansion of macro âcontainer_ofâ
> >   struct st *x = container_of(p, struct st, b);
> >                  ^
> > -------------------------------------------------------
> >
> > Replace the type checking performed by the macro to avoid these
> > warnings.  Make sure `*(ptr)` either has type compatible with the
> > member, or has type compatible with `void`, ignoring qualifiers.  Raise
> > compiler errors if this is not true.  This is stronger than the previous
> > behaviour, which only resulted in compiler warnings for a type mismatch.
> >
> > Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Michal Nazarewicz <mina86@xxxxxxxxxx>
> > Cc: Hidehiro Kawai <hidehiro.kawai.ez@xxxxxxxxxxx>
> > Cc: Borislav Petkov <bp@xxxxxxx>
> > Cc: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
> > Cc: Johannes Berg <johannes.berg@xxxxxxxxx>
> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > Cc: Alexander Potapenko <glider@xxxxxxxxxx>
> > Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
>
> Seems reasonable to me. I think this actually improves the errors
> reported when something is mismatched in container_of(). Silly
> question: does this pass an allyesconfig?

Yes for ARCH=i386 at least.

> Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>

Thanks!

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx> )=-
-=(                          Web: http://www.mev.co.uk/  )=-