From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S969691AbdEYOHw (ORCPT ); Thu, 25 May 2017 10:07:52 -0400 Received: from mail-wm0-f44.google.com ([74.125.82.44]:35612 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935770AbdEYOHQ (ORCPT ); Thu, 25 May 2017 10:07:16 -0400 From: Michal Nazarewicz To: Ian Abbott , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org Cc: Alexander Potapenko , Andrew Morton , Arnd Bergmann , Borislav Petkov , Hidehiro Kawai , Ian Abbott , Jakub Kicinski , Johannes Berg , Kees Cook , Peter Zijlstra , Rasmus Villemoes , Steven Rostedt Subject: Re: [PATCH v5 6/6] kernel.h: handle pointers to arrays better in container_of() In-Reply-To: <20170525120316.24473-7-abbotti@mev.co.uk> Organization: http://mina86.com/ References: <20170525120316.24473-1-abbotti@mev.co.uk> <20170525120316.24473-7-abbotti@mev.co.uk> Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAJFBMVEWbfGlUPDDHgE57V0jUupKjgIObY0PLrom9mH4dFRK4gmjPs41MxjOgAAACP0lEQVQ4T23Sv2vbQBQHcBk1xE6WyALX107VUEgmn6+ouUwpEQQ6uRjttkWP4CkBg2M0BQLBdPFZYPsyFYo7qEtKDQ7on+t7+nF2Ux8ahD587717OmNYrOvycHsZ+o2r051wHTHysAvGb8ygvgu4QWT0sCmkgZCIEnlV2X8BtyraazFGDuxhmKSQJMlwHQ7v5MHSNxmz78rfElwAa3ieVD9e+hBhjaPDDG6NgFo2f4wBMNIo5YmRtF0RyDgFjJjlMIWbnuM4x9MMfABGTlN4qgIQB4A1DEyA1BHWtfeWNUMwiVJKoqh97KrkOO+qzgluVYLvFCUKAX73nONeBr7BGMdM6Sg0kuep03VywLaIzRiVr+GAzKlpQIsAFnWAG2e6DT5WmWDiudZMIc6hYrMOmeMQK9WX0B+/RfjzL9DI7Y9/Iayn29Ci0r2i4f9gMimMSZLCDMalgQGU5hnUtqAN0OGvEmO1Wnl0C0wWSCEHnuHBqmygxdxA8oWXwbipoc1EoNR9DqOpBpOJrnr0criQab9ZT4LL+wI+K7GBQH30CrhUruilgP9DRTrhVWZCiAyILP+wiuLeCKGTD6r/nc8LOJcAwR6IBTUs+7CASw3QFZ0MdA2PI3zNziH4ZKVhXCRMBjeZ1DWMekKwDCASwExy+NQ86TaykaDAFHO4aP48y4fIcDM5yOG8GcTLbOyp8A8azjJI93JFd1EA6yN8sSxMQJWoABqniRZVykYgRXErzrdqExAoUrRb0xfRp8p2A/4XmfilTtkDZ4cAAAAASUVORK5CYII= X-Face: -TR8(rDTHy/(xl?SfWd1|3:TTgDIatE^t'vop%*gVg[kn$t{EpK(P"VQ=~T2#ysNmJKN$"yTRLB4YQs$4{[.]Fc1)*O]3+XO^oXM>Q#b^ix,O)Zbn)q[y06$`e3?C)`CwR9y5riE=fv^X@x$y?D:XO6L&x4f-}}I4=VRNwiA^t1-ZrVK^07.Pi/57c_du'& OpenPGP: id=AC1F5F5CD41888F8CC8458582060401250751FF4; url=http://mina86.com/mina86.pub X-Hashcash: 1:20:170525:johannes.berg@intel.com::c1Dc13l1Xm8L0HW3:0000000000000000000000000000000000000008d6 X-Hashcash: 1:20:170525:linux-arch@vger.kernel.org::KWg64pJ9QOgtkOv4:0000000000000000000000000000000000006Ua X-Hashcash: 1:20:170525:linux-kernel@vger.kernel.org::XCEXzqBCfgdGXD7p:0000000000000000000000000000000000E4k X-Hashcash: 1:20:170525:akpm@linux-foundation.org::46oVnCQzHOZHDWnp:0000000000000000000000000000000000000uye X-Hashcash: 1:20:170525:keescook@chromium.org::dEg7N3FUzpVbIhIf:000000000000000000000000000000000000000015W+ X-Hashcash: 1:20:170525:arnd@arndb.de::N/I5kmyhlBZ1NCu7:00002JZl X-Hashcash: 1:20:170525:rostedt@goodmis.org::Sm5AV878yYq5m8jC:00000000000000000000000000000000000000000025G1 X-Hashcash: 1:20:170525:peterz@infradead.org::VXmYQBgOUd0Bz8m9:000000000000000000000000000000000000000002ckV X-Hashcash: 1:20:170525:bp@suse.de::wDICT9C1wGp12XLI:000000034dz X-Hashcash: 1:20:170525:linux@rasmusvillemoes.dk::TCb48EiXfAoMzkNh:00000000000000000000000000000000000005JfH X-Hashcash: 1:20:170525:abbotti@mev.co.uk::22pzp0FCBLjB2DOB:000000000000000000000000000000000000000000009aZN X-Hashcash: 1:20:170525:abbotti@mev.co.uk::3PFQQsZWxxbuBE2D:000000000000000000000000000000000000000000008hbB X-Hashcash: 1:20:170525:hidehiro.kawai.ez@hitachi.com::TZOPLYgI+GDpxFBn:000000000000000000000000000000008vvV X-Hashcash: 1:20:170525:glider@google.com::fcl5oq9pSQo1WKeC:00000000000000000000000000000000000000000000BcS7 X-Hashcash: 1:20:170525:jakub.kicinski@netronome.com::EDXikpH5xAjx+x1g:000000000000000000000000000000000DRZU Date: Thu, 25 May 2017 16:07:12 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v4PE9f3s014291 On Thu, May 25 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 = { .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 > 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 > Acked-by: Michal Nazarewicz > --- > 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. > v5: Added Acked-by for Michal Nazarewicz. Included > instead of to avoid a circular dependency that resulted in > build failures when was included before . > --- > 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..1c9c11c9f1a8 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -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 = (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))); }) > > /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */ > #ifdef CONFIG_FTRACE_MCOUNT_RECORD -- Best regards ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ «If at first you don’t succeed, give up skydiving» From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Nazarewicz Subject: Re: [PATCH v5 6/6] kernel.h: handle pointers to arrays better in container_of() Date: Thu, 25 May 2017 16:07:12 +0200 Message-ID: References: <20170525120316.24473-1-abbotti@mev.co.uk> <20170525120316.24473-7-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-f41.google.com ([74.125.82.41]:36706 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935611AbdEYOHQ (ORCPT ); Thu, 25 May 2017 10:07:16 -0400 Received: by mail-wm0-f41.google.com with SMTP id 7so93096183wmo.1 for ; Thu, 25 May 2017 07:07:15 -0700 (PDT) In-Reply-To: <20170525120316.24473-7-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: Alexander Potapenko , Andrew Morton , Arnd Bergmann , Borislav Petkov , Hidehiro Kawai , Ian Abbott , Jakub Kicinski , Johannes Berg , Kees Cook , Peter Zijlstra , Rasmus Villemoes , Steven Rostedt On Thu, May 25 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 > Acked-by: Michal Nazarewicz > --- > 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. > v5: Added Acked-by for Michal Nazarewicz. Included > instead of to avoid a circular dependency that resulted in > build failures when was included before . > --- > 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..1c9c11c9f1a8 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-f41.google.com ([74.125.82.41]:36706 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935611AbdEYOHQ (ORCPT ); Thu, 25 May 2017 10:07:16 -0400 Received: by mail-wm0-f41.google.com with SMTP id 7so93096183wmo.1 for ; Thu, 25 May 2017 07:07:15 -0700 (PDT) From: Michal Nazarewicz Subject: Re: [PATCH v5 6/6] kernel.h: handle pointers to arrays better in container_of() In-Reply-To: <20170525120316.24473-7-abbotti@mev.co.uk> References: <20170525120316.24473-1-abbotti@mev.co.uk> <20170525120316.24473-7-abbotti@mev.co.uk> Date: Thu, 25 May 2017 16:07:12 +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: Alexander Potapenko , Andrew Morton , Arnd Bergmann , Borislav Petkov , Hidehiro Kawai , Jakub Kicinski , Johannes Berg , Kees Cook , Peter Zijlstra , Rasmus Villemoes , Steven Rostedt Message-ID: <20170525140712.mr_Mg4a9nQPxI_kjqrD6_z_nZn1LuRozvFNiOECHNAQ@z> On Thu, May 25 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 > Acked-by: Michal Nazarewicz > --- > 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. > v5: Added Acked-by for Michal Nazarewicz. Included > instead of to avoid a circular dependency that resulted in > build failures when was included before . > --- > 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..1c9c11c9f1a8 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