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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935871AbdEWSpI (ORCPT ); Tue, 23 May 2017 14:45:08 -0400 Received: from mail-wm0-f47.google.com ([74.125.82.47]:34430 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935039AbdEWSpD (ORCPT ); Tue, 23 May 2017 14:45:03 -0400 From: Michal Nazarewicz 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 , Ian Abbott 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> Organization: http://mina86.com/ References: <20170523160112.3446-1-abbotti@mev.co.uk> <20170523160112.3446-3-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:170523:glider@google.com::dHQEzIOsFx6K5cD4:000000000000000000000000000000000000000000000Ern X-Hashcash: 1:20:170523:hidehiro.kawai.ez@hitachi.com::YcvA56Tm0YIGs362:000000000000000000000000000000000jt9 X-Hashcash: 1:20:170523:linux-arch@vger.kernel.org::oFSfO1y3ABNz+CX3:000000000000000000000000000000000000QUB X-Hashcash: 1:20:170523:linux-kernel@vger.kernel.org::+Ia+RrOBLm9IUjN/:0000000000000000000000000000000002agJ X-Hashcash: 1:20:170523:bp@suse.de::A4OyQWQG/h5BEqcE:000000034Ne X-Hashcash: 1:20:170523:arnd@arndb.de::jwu8AHwge9qbTnJh:00005Qss X-Hashcash: 1:20:170523:linux@rasmusvillemoes.dk::X4FQQhJYRZnXjA65:00000000000000000000000000000000000004Vf3 X-Hashcash: 1:20:170523:abbotti@mev.co.uk::RlDXyUrUkUYX+1fL:0000000000000000000000000000000000000000000048Xy X-Hashcash: 1:20:170523:abbotti@mev.co.uk::2x1y3bbT9gSsz+Nk:000000000000000000000000000000000000000000004807 X-Hashcash: 1:20:170523:johannes.berg@intel.com::9feiR8oCAzDZALyp:000000000000000000000000000000000000005u6H X-Hashcash: 1:20:170523:akpm@linux-foundation.org::hfjzE1ocPTRDu8lP:0000000000000000000000000000000000006A7b X-Hashcash: 1:20:170523:peterz@infradead.org::Ft3ZVngJ2oPo+gH+:000000000000000000000000000000000000000004x5g Date: Tue, 23 May 2017 20:44:59 +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 v4NIjDYW021342 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 = { .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 > --- > 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 > > @@ -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»