All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] kernel.h: container_of() pointer checking
@ 2017-05-23 15:53 Ian Abbott
  2017-05-23 15:53 ` [PATCH v3 1/2] asm-generic/bug.h: declare struct pt_regs; before function prototype Ian Abbott
  2017-05-23 15:53 ` [PATCH v3 2/2] kernel.h: handle pointers to arrays better in container_of() Ian Abbott
  0 siblings, 2 replies; 6+ messages in thread
From: Ian Abbott @ 2017-05-23 15:53 UTC (permalink / raw)
  To: linux-kernel, linux-arch
  Cc: Arnd Bergmann, Andrew Morton, Michal Nazarewicz, Hidehiro Kawai,
	Borislav Petkov, Rasmus Villemoes, Johannes Berg, Peter Zijlstra,
	Alexander Potapenko

[Apologies for the resend.  I got both mailing list addresses wrong!]

Patch 2 changes the container_of() macro to improve the compatibility
checking when the member has array type.  As a bonus (?), if the pointer
neither points to a type compatible with the member nor points to a type
compatible with void, compiler errors are produced instead of warnings.

Patch 1 is a prerequisite to avoid a lot of warnings when <linux/bug.h>
is included by <linux/kernel.h>.

1) asm-generic/bug.h: declare struct pt_regs; before function prototype
2) kernel.h: handle pointers to arrays better in container_of()

 include/asm-generic/bug.h | 1 +
 include/linux/kernel.h    | 9 ++++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 1/2] asm-generic/bug.h: declare struct pt_regs; before function prototype
  2017-05-23 15:53 [PATCH v3 0/2] kernel.h: container_of() pointer checking Ian Abbott
@ 2017-05-23 15:53 ` Ian Abbott
  2017-05-23 15:53 ` [PATCH v3 2/2] kernel.h: handle pointers to arrays better in container_of() Ian Abbott
  1 sibling, 0 replies; 6+ messages in thread
From: Ian Abbott @ 2017-05-23 15:53 UTC (permalink / raw)
  To: linux-kernel, linux-arch
  Cc: Arnd Bergmann, Andrew Morton, Michal Nazarewicz, Hidehiro Kawai,
	Borislav Petkov, Rasmus Villemoes, Johannes Berg, Peter Zijlstra,
	Alexander Potapenko, Ian Abbott

The declaration of `__warn()` has `struct pt_regs *regs` as one of its
parameters.  This can result in compiler warnings if `struct regs` is
not already declared.  Add an empty declaration of `struct pt_regs` to
avoid the warnings.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Acked-by: Acked-by: Arnd Bergmann <arnd@arndb.de>
---
v3: Actually, there was no v1 or v2.  I called this v3 to match the
series.
---
 include/asm-generic/bug.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index d6f4aed479a1..87191357d303 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -97,6 +97,7 @@ extern void warn_slowpath_null(const char *file, const int line);
 
 /* used internally by panic.c */
 struct warn_args;
+struct pt_regs;
 
 void __warn(const char *file, int line, void *caller, unsigned taint,
 	    struct pt_regs *regs, struct warn_args *args);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3 2/2] kernel.h: handle pointers to arrays better in container_of()
  2017-05-23 15:53 [PATCH v3 0/2] kernel.h: container_of() pointer checking Ian Abbott
  2017-05-23 15:53 ` [PATCH v3 1/2] asm-generic/bug.h: declare struct pt_regs; before function prototype Ian Abbott
@ 2017-05-23 15:53 ` Ian Abbott
  2017-05-24 14:29     ` kbuild test robot
  1 sibling, 1 reply; 6+ messages in thread
From: Ian Abbott @ 2017-05-23 15:53 UTC (permalink / raw)
  To: linux-kernel, linux-arch
  Cc: Arnd Bergmann, Andrew Morton, Michal Nazarewicz, Hidehiro Kawai,
	Borislav Petkov, Rasmus Villemoes, Johannes Berg, Peter Zijlstra,
	Alexander Potapenko, Ian Abbott

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@mev.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Johannes Berg <johannes.berg@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alexander Potapenko <glider@google.com>
---
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.
---
 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 <linux/log2.h>
 #include <linux/typecheck.h>
 #include <linux/printk.h>
+#include <linux/bug.h>
 #include <asm/byteorder.h>
 #include <uapi/linux/kernel.h>
 
@@ -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
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] kernel.h: handle pointers to arrays better in container_of()
  2017-05-23 15:53 ` [PATCH v3 2/2] kernel.h: handle pointers to arrays better in container_of() Ian Abbott
  2017-05-24 14:29     ` kbuild test robot
@ 2017-05-24 14:29     ` kbuild test robot
  0 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2017-05-24 14:29 UTC (permalink / raw)
  To: Ian Abbott
  Cc: kbuild-all, linux-kernel, linux-arch, Arnd Bergmann,
	Andrew Morton, Michal Nazarewicz, Hidehiro Kawai,
	Borislav Petkov, Rasmus Villemoes, Johannes Berg, Peter Zijlstra,
	Alexander Potapenko, Ian Abbott

[-- Attachment #1: Type: text/plain, Size: 2573 bytes --]

Hi Ian,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.12-rc2 next-20170524]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ian-Abbott/kernel-h-container_of-pointer-checking/20170524-071605
config: x86_64-randconfig-n0-05242055 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from include/asm-generic/bug.h:15,
                    from arch/x86/include/asm/bug.h:81,
                    from drivers/block/drbd/drbd_interval.c:1:
   include/linux/bug.h:103:47: warning: 'struct bug_entry' declared inside parameter list [enabled by default]
    static inline int is_warning_bug(const struct bug_entry *bug)
                                                  ^
   include/linux/bug.h:103:47: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
   include/linux/bug.h: In function 'is_warning_bug':
>> include/linux/bug.h:105:12: error: dereferencing pointer to incomplete type
     return bug->flags & BUGFLAG_WARNING;
               ^

vim +105 include/linux/bug.h

ff20c2e0 Kirill A. Shutemov  2016-03-01   97  
35edd910 Paul Gortmaker      2011-11-16   98  #endif	/* __CHECKER__ */
35edd910 Paul Gortmaker      2011-11-16   99  
7664c5a1 Jeremy Fitzhardinge 2006-12-08  100  #ifdef CONFIG_GENERIC_BUG
7664c5a1 Jeremy Fitzhardinge 2006-12-08  101  #include <asm-generic/bug.h>
7664c5a1 Jeremy Fitzhardinge 2006-12-08  102  
7664c5a1 Jeremy Fitzhardinge 2006-12-08 @103  static inline int is_warning_bug(const struct bug_entry *bug)
7664c5a1 Jeremy Fitzhardinge 2006-12-08  104  {
7664c5a1 Jeremy Fitzhardinge 2006-12-08 @105  	return bug->flags & BUGFLAG_WARNING;
7664c5a1 Jeremy Fitzhardinge 2006-12-08  106  }
7664c5a1 Jeremy Fitzhardinge 2006-12-08  107  
19d43626 Peter Zijlstra      2017-02-25  108  struct bug_entry *find_bug(unsigned long bugaddr);

:::::: The code at line 105 was first introduced by commit
:::::: 7664c5a1da4711bb6383117f51b94c8dc8f3f1cd [PATCH] Generic BUG implementation

:::::: TO: Jeremy Fitzhardinge <jeremy@goop.org>
:::::: CC: Linus Torvalds <torvalds@woody.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25546 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] kernel.h: handle pointers to arrays better in container_of()
@ 2017-05-24 14:29     ` kbuild test robot
  0 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2017-05-24 14:29 UTC (permalink / raw)
  Cc: kbuild-all, linux-kernel, linux-arch, Arnd Bergmann,
	Andrew Morton, Michal Nazarewicz, Hidehiro Kawai,
	Borislav Petkov, Rasmus Villemoes, Johannes Berg, Peter Zijlstra,
	Alexander Potapenko, Ian Abbott

[-- Attachment #1: Type: text/plain, Size: 2573 bytes --]

Hi Ian,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.12-rc2 next-20170524]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ian-Abbott/kernel-h-container_of-pointer-checking/20170524-071605
config: x86_64-randconfig-n0-05242055 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from include/asm-generic/bug.h:15,
                    from arch/x86/include/asm/bug.h:81,
                    from drivers/block/drbd/drbd_interval.c:1:
   include/linux/bug.h:103:47: warning: 'struct bug_entry' declared inside parameter list [enabled by default]
    static inline int is_warning_bug(const struct bug_entry *bug)
                                                  ^
   include/linux/bug.h:103:47: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
   include/linux/bug.h: In function 'is_warning_bug':
>> include/linux/bug.h:105:12: error: dereferencing pointer to incomplete type
     return bug->flags & BUGFLAG_WARNING;
               ^

vim +105 include/linux/bug.h

ff20c2e0 Kirill A. Shutemov  2016-03-01   97  
35edd910 Paul Gortmaker      2011-11-16   98  #endif	/* __CHECKER__ */
35edd910 Paul Gortmaker      2011-11-16   99  
7664c5a1 Jeremy Fitzhardinge 2006-12-08  100  #ifdef CONFIG_GENERIC_BUG
7664c5a1 Jeremy Fitzhardinge 2006-12-08  101  #include <asm-generic/bug.h>
7664c5a1 Jeremy Fitzhardinge 2006-12-08  102  
7664c5a1 Jeremy Fitzhardinge 2006-12-08 @103  static inline int is_warning_bug(const struct bug_entry *bug)
7664c5a1 Jeremy Fitzhardinge 2006-12-08  104  {
7664c5a1 Jeremy Fitzhardinge 2006-12-08 @105  	return bug->flags & BUGFLAG_WARNING;
7664c5a1 Jeremy Fitzhardinge 2006-12-08  106  }
7664c5a1 Jeremy Fitzhardinge 2006-12-08  107  
19d43626 Peter Zijlstra      2017-02-25  108  struct bug_entry *find_bug(unsigned long bugaddr);

:::::: The code at line 105 was first introduced by commit
:::::: 7664c5a1da4711bb6383117f51b94c8dc8f3f1cd [PATCH] Generic BUG implementation

:::::: TO: Jeremy Fitzhardinge <jeremy@goop.org>
:::::: CC: Linus Torvalds <torvalds@woody.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25546 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] kernel.h: handle pointers to arrays better in container_of()
@ 2017-05-24 14:29     ` kbuild test robot
  0 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2017-05-24 14:29 UTC (permalink / raw)
  To: Ian Abbott
  Cc: kbuild-all, linux-kernel, linux-arch, Arnd Bergmann,
	Andrew Morton, Michal Nazarewicz, Hidehiro Kawai,
	Borislav Petkov, Rasmus Villemoes, Johannes Berg, Peter Zijlstra,
	Alexander Potapenko

[-- Attachment #1: Type: text/plain, Size: 2573 bytes --]

Hi Ian,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.12-rc2 next-20170524]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ian-Abbott/kernel-h-container_of-pointer-checking/20170524-071605
config: x86_64-randconfig-n0-05242055 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from include/asm-generic/bug.h:15,
                    from arch/x86/include/asm/bug.h:81,
                    from drivers/block/drbd/drbd_interval.c:1:
   include/linux/bug.h:103:47: warning: 'struct bug_entry' declared inside parameter list [enabled by default]
    static inline int is_warning_bug(const struct bug_entry *bug)
                                                  ^
   include/linux/bug.h:103:47: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
   include/linux/bug.h: In function 'is_warning_bug':
>> include/linux/bug.h:105:12: error: dereferencing pointer to incomplete type
     return bug->flags & BUGFLAG_WARNING;
               ^

vim +105 include/linux/bug.h

ff20c2e0 Kirill A. Shutemov  2016-03-01   97  
35edd910 Paul Gortmaker      2011-11-16   98  #endif	/* __CHECKER__ */
35edd910 Paul Gortmaker      2011-11-16   99  
7664c5a1 Jeremy Fitzhardinge 2006-12-08  100  #ifdef CONFIG_GENERIC_BUG
7664c5a1 Jeremy Fitzhardinge 2006-12-08  101  #include <asm-generic/bug.h>
7664c5a1 Jeremy Fitzhardinge 2006-12-08  102  
7664c5a1 Jeremy Fitzhardinge 2006-12-08 @103  static inline int is_warning_bug(const struct bug_entry *bug)
7664c5a1 Jeremy Fitzhardinge 2006-12-08  104  {
7664c5a1 Jeremy Fitzhardinge 2006-12-08 @105  	return bug->flags & BUGFLAG_WARNING;
7664c5a1 Jeremy Fitzhardinge 2006-12-08  106  }
7664c5a1 Jeremy Fitzhardinge 2006-12-08  107  
19d43626 Peter Zijlstra      2017-02-25  108  struct bug_entry *find_bug(unsigned long bugaddr);

:::::: The code at line 105 was first introduced by commit
:::::: 7664c5a1da4711bb6383117f51b94c8dc8f3f1cd [PATCH] Generic BUG implementation

:::::: TO: Jeremy Fitzhardinge <jeremy@goop.org>
:::::: CC: Linus Torvalds <torvalds@woody.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25546 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-24 14:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-23 15:53 [PATCH v3 0/2] kernel.h: container_of() pointer checking Ian Abbott
2017-05-23 15:53 ` [PATCH v3 1/2] asm-generic/bug.h: declare struct pt_regs; before function prototype Ian Abbott
2017-05-23 15:53 ` [PATCH v3 2/2] kernel.h: handle pointers to arrays better in container_of() Ian Abbott
2017-05-24 14:29   ` kbuild test robot
2017-05-24 14:29     ` kbuild test robot
2017-05-24 14:29     ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.