From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Nazarewicz Subject: Re: [PATCH v4 2/2] kernel.h: handle pointers to arrays better in container_of() Date: Tue, 23 May 2017 20:44:59 +0200 Message-ID: References: <20170523160112.3446-1-abbotti@mev.co.uk> <20170523160112.3446-3-abbotti@mev.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wm0-f49.google.com ([74.125.82.49]:38332 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934085AbdEWSpD (ORCPT ); Tue, 23 May 2017 14:45:03 -0400 Received: by mail-wm0-f49.google.com with SMTP id e127so42064170wmg.1 for ; Tue, 23 May 2017 11:45:02 -0700 (PDT) In-Reply-To: <20170523160112.3446-3-abbotti@mev.co.uk> Sender: linux-arch-owner@vger.kernel.org List-ID: To: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org Cc: Arnd Bergmann , Andrew Morton , Hidehiro Kawai , Borislav Petkov , Rasmus Villemoes , Johannes Berg , Peter Zijlstra , Alexander Potapenko , Ian Abbott On Tue, May 23 2017, Ian Abbott 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 > #include > > MODULE_LICENSE("GPL"); > > struct st { > int a; > char b[16]; > }; > > static int __init example_init(void) { > struct st t =3D { .a =3D 101, .b =3D "hello" }; > char (*p)[16] =3D &t.b; > struct st *x =3D 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 =E2=80=98example_init=E2=80=99: > {k}/include/linux/kernel.h:854:48: warning: initialization from > incompatible pointer type > const typeof( ((type *)0)->member ) *__mptr =3D (ptr); \ > ^ > {m}/example.c:14:17: note: in expansion of macro =E2=80=98container_of=E2= =80=99 > struct st *x =3D container_of(p, struct st, b); > ^ > {k}/include/linux/kernel.h:854:48: warning: (near initialization for > =E2=80=98x=E2=80=99) > const typeof( ((type *)0)->member ) *__mptr =3D (ptr); \ > ^ > {m}/example.c:14:17: note: in expansion of macro =E2=80=98container_of=E2= =80=99 > struct st *x =3D 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 > Cc: Andrew Morton > Cc: Michal Nazarewicz Acked-by: Michal Nazarewicz > Cc: Hidehiro Kawai > Cc: Borislav Petkov > Cc: Rasmus Villemoes > Cc: Johannes Berg > Cc: Peter Zijlstra > Cc: Alexander Potapenko > --- > v2: Rebased and altered description to provide an example of when the > compiler warnings occur. v1 (from 2016-10-10) also modified a > 'container_of_safe()' macro that never made it out of "linux-next". > v3: Added back some type checking at the suggestion of Michal > Nazarewicz with some helpful hints by Peter Zijlstra. > v4: No change. > --- > include/linux/kernel.h | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index 13bc08aba704..f5dba37aaa60 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > #include > #include >=20=20 > @@ -850,9 +851,11 @@ static inline void ftrace_dump(enum ftrace_dump_mode= oops_dump_mode) { } > * @member: the name of the member within the struct. > * > */ > -#define container_of(ptr, type, member) ({ \ > - const typeof( ((type *)0)->member ) *__mptr =3D (ptr); \ > - (type *)( (char *)__mptr - offsetof(type,member) );}) > +#define container_of(ptr, type, member) ({ \ > + BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ > + !__same_type(*(ptr), void), \ > + "pointer type mismatch in container_of()"); \ > + ((type *)((char *)(ptr) - offsetof(type, member))); }) >=20=20 > /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */ > #ifdef CONFIG_FTRACE_MCOUNT_RECORD --=20 Best regards =E3=83=9F=E3=83=8F=E3=82=A6 =E2=80=9C=F0=9D=93=B6=F0=9D=93=B2=F0=9D=93=B7= =F0=9D=93=AA86=E2=80=9D =E3=83=8A=E3=82=B6=E3=83=AC=E3=83=B4=E3=82=A4=E3=83= =84 =C2=ABIf at first you don=E2=80=99t succeed, give up skydiving=C2=BB From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f49.google.com ([74.125.82.49]:38332 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934085AbdEWSpD (ORCPT ); Tue, 23 May 2017 14:45:03 -0400 Received: by mail-wm0-f49.google.com with SMTP id e127so42064170wmg.1 for ; Tue, 23 May 2017 11:45:02 -0700 (PDT) From: Michal Nazarewicz Subject: Re: [PATCH v4 2/2] kernel.h: handle pointers to arrays better in container_of() In-Reply-To: <20170523160112.3446-3-abbotti@mev.co.uk> References: <20170523160112.3446-1-abbotti@mev.co.uk> <20170523160112.3446-3-abbotti@mev.co.uk> Date: Tue, 23 May 2017 20:44:59 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Sender: linux-arch-owner@vger.kernel.org List-ID: To: Ian Abbott , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org Cc: Arnd Bergmann , Andrew Morton , Hidehiro Kawai , Borislav Petkov , Rasmus Villemoes , Johannes Berg , Peter Zijlstra , Alexander Potapenko Message-ID: <20170523184459.-6WZYuIeQVmVZ4XlCZmEGJgOXuS2LUzAa9hYviM6nKg@z> On Tue, May 23 2017, Ian Abbott 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 > #include > > MODULE_LICENSE("GPL"); > > struct st { > int a; > char b[16]; > }; > > static int __init example_init(void) { > struct st t =3D { .a =3D 101, .b =3D "hello" }; > char (*p)[16] =3D &t.b; > struct st *x =3D 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 =E2=80=98example_init=E2=80=99: > {k}/include/linux/kernel.h:854:48: warning: initialization from > incompatible pointer type > const typeof( ((type *)0)->member ) *__mptr =3D (ptr); \ > ^ > {m}/example.c:14:17: note: in expansion of macro =E2=80=98container_of=E2= =80=99 > struct st *x =3D container_of(p, struct st, b); > ^ > {k}/include/linux/kernel.h:854:48: warning: (near initialization for > =E2=80=98x=E2=80=99) > const typeof( ((type *)0)->member ) *__mptr =3D (ptr); \ > ^ > {m}/example.c:14:17: note: in expansion of macro =E2=80=98container_of=E2= =80=99 > struct st *x =3D 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 > Cc: Andrew Morton > Cc: Michal Nazarewicz Acked-by: Michal Nazarewicz > Cc: Hidehiro Kawai > Cc: Borislav Petkov > Cc: Rasmus Villemoes > Cc: Johannes Berg > Cc: Peter Zijlstra > Cc: Alexander Potapenko > --- > v2: Rebased and altered description to provide an example of when the > compiler warnings occur. v1 (from 2016-10-10) also modified a > 'container_of_safe()' macro that never made it out of "linux-next". > v3: Added back some type checking at the suggestion of Michal > Nazarewicz with some helpful hints by Peter Zijlstra. > v4: No change. > --- > include/linux/kernel.h | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index 13bc08aba704..f5dba37aaa60 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > #include > #include >=20=20 > @@ -850,9 +851,11 @@ static inline void ftrace_dump(enum ftrace_dump_mode= oops_dump_mode) { } > * @member: the name of the member within the struct. > * > */ > -#define container_of(ptr, type, member) ({ \ > - const typeof( ((type *)0)->member ) *__mptr =3D (ptr); \ > - (type *)( (char *)__mptr - offsetof(type,member) );}) > +#define container_of(ptr, type, member) ({ \ > + BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ > + !__same_type(*(ptr), void), \ > + "pointer type mismatch in container_of()"); \ > + ((type *)((char *)(ptr) - offsetof(type, member))); }) >=20=20 > /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */ > #ifdef CONFIG_FTRACE_MCOUNT_RECORD --=20 Best regards =E3=83=9F=E3=83=8F=E3=82=A6 =E2=80=9C=F0=9D=93=B6=F0=9D=93=B2=F0=9D=93=B7= =F0=9D=93=AA86=E2=80=9D =E3=83=8A=E3=82=B6=E3=83=AC=E3=83=B4=E3=82=A4=E3=83= =84 =C2=ABIf at first you don=E2=80=99t succeed, give up skydiving=C2=BB