* [PATCH] kcsan: fix debugfs initcall return type @ 2021-05-14 14:00 Arnd Bergmann 2021-05-14 14:10 ` Greg Kroah-Hartman 2021-05-14 18:29 ` Nathan Chancellor 0 siblings, 2 replies; 17+ messages in thread From: Arnd Bergmann @ 2021-05-14 14:00 UTC (permalink / raw) To: Marco Elver, Nathan Chancellor, Nick Desaulniers, Paul E. McKenney, Greg Kroah-Hartman Cc: Arnd Bergmann, Dmitry Vyukov, kasan-dev, linux-kernel, clang-built-linux From: Arnd Bergmann <arnd@arndb.de> clang points out that an initcall funciton should return an 'int': kernel/kcsan/debugfs.c:274:15: error: returning 'void' from a function with incompatible result type 'int' late_initcall(kcsan_debugfs_init); ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ include/linux/init.h:292:46: note: expanded from macro 'late_initcall' #define late_initcall(fn) __define_initcall(fn, 7) Fixes: e36299efe7d7 ("kcsan, debugfs: Move debugfs file creation out of early init") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- kernel/kcsan/debugfs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c index c1dd02f3be8b..e65de172ccf7 100644 --- a/kernel/kcsan/debugfs.c +++ b/kernel/kcsan/debugfs.c @@ -266,9 +266,10 @@ static const struct file_operations debugfs_ops = .release = single_release }; -static void __init kcsan_debugfs_init(void) +static int __init kcsan_debugfs_init(void) { debugfs_create_file("kcsan", 0644, NULL, NULL, &debugfs_ops); + return 0; } late_initcall(kcsan_debugfs_init); -- 2.29.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 14:00 [PATCH] kcsan: fix debugfs initcall return type Arnd Bergmann @ 2021-05-14 14:10 ` Greg Kroah-Hartman 2021-05-14 14:45 ` Marco Elver 2021-05-14 18:29 ` Nathan Chancellor 1 sibling, 1 reply; 17+ messages in thread From: Greg Kroah-Hartman @ 2021-05-14 14:10 UTC (permalink / raw) To: Arnd Bergmann Cc: Marco Elver, Nathan Chancellor, Nick Desaulniers, Paul E. McKenney, Arnd Bergmann, Dmitry Vyukov, kasan-dev, linux-kernel, clang-built-linux On Fri, May 14, 2021 at 04:00:08PM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > clang points out that an initcall funciton should return an 'int': > > kernel/kcsan/debugfs.c:274:15: error: returning 'void' from a function with incompatible result type 'int' > late_initcall(kcsan_debugfs_init); > ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ > include/linux/init.h:292:46: note: expanded from macro 'late_initcall' > #define late_initcall(fn) __define_initcall(fn, 7) > > Fixes: e36299efe7d7 ("kcsan, debugfs: Move debugfs file creation out of early init") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > kernel/kcsan/debugfs.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c > index c1dd02f3be8b..e65de172ccf7 100644 > --- a/kernel/kcsan/debugfs.c > +++ b/kernel/kcsan/debugfs.c > @@ -266,9 +266,10 @@ static const struct file_operations debugfs_ops = > .release = single_release > }; > > -static void __init kcsan_debugfs_init(void) > +static int __init kcsan_debugfs_init(void) > { > debugfs_create_file("kcsan", 0644, NULL, NULL, &debugfs_ops); > + return 0; > } > > late_initcall(kcsan_debugfs_init); > -- > 2.29.2 > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 14:10 ` Greg Kroah-Hartman @ 2021-05-14 14:45 ` Marco Elver 2021-05-14 15:28 ` Arnd Bergmann 2021-05-14 18:40 ` Nathan Chancellor 0 siblings, 2 replies; 17+ messages in thread From: Marco Elver @ 2021-05-14 14:45 UTC (permalink / raw) To: Greg Kroah-Hartman, Paul E. McKenney Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Arnd Bergmann, Dmitry Vyukov, kasan-dev, LKML, clang-built-linux On Fri, 14 May 2021 at 16:10, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Fri, May 14, 2021 at 04:00:08PM +0200, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > > > clang points out that an initcall funciton should return an 'int': > > > > kernel/kcsan/debugfs.c:274:15: error: returning 'void' from a function with incompatible result type 'int' > > late_initcall(kcsan_debugfs_init); > > ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ > > include/linux/init.h:292:46: note: expanded from macro 'late_initcall' > > #define late_initcall(fn) __define_initcall(fn, 7) > > > > Fixes: e36299efe7d7 ("kcsan, debugfs: Move debugfs file creation out of early init") > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> [...] > > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reviewed-by: Marco Elver <elver@google.com> Thanks for catching this -- it boggles my mind why gcc nor clang wouldn't warn about this by default... Is this a new clang? Paul, please also add a "Cc: stable <stable@vger.kernel.org>" because e36299efe7d7 is, too. Thanks, -- Marco ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 14:45 ` Marco Elver @ 2021-05-14 15:28 ` Arnd Bergmann 2021-05-14 18:40 ` Nathan Chancellor 1 sibling, 0 replies; 17+ messages in thread From: Arnd Bergmann @ 2021-05-14 15:28 UTC (permalink / raw) To: Marco Elver Cc: Greg Kroah-Hartman, Paul E. McKenney, Nathan Chancellor, Nick Desaulniers, Dmitry Vyukov, kasan-dev, LKML, clang-built-linux On Fri, May 14, 2021 at 4:45 PM 'Marco Elver' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > On Fri, 14 May 2021 at 16:10, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > On Fri, May 14, 2021 at 04:00:08PM +0200, Arnd Bergmann wrote: > > > From: Arnd Bergmann <arnd@arndb.de> > > > > > > clang points out that an initcall funciton should return an 'int': > > > > > > kernel/kcsan/debugfs.c:274:15: error: returning 'void' from a function with incompatible result type 'int' > > > late_initcall(kcsan_debugfs_init); > > > ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ > > > include/linux/init.h:292:46: note: expanded from macro 'late_initcall' > > > #define late_initcall(fn) __define_initcall(fn, 7) > > > > > > Fixes: e36299efe7d7 ("kcsan, debugfs: Move debugfs file creation out of early init") > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > [...] > > > > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Reviewed-by: Marco Elver <elver@google.com> > > Thanks for catching this -- it boggles my mind why gcc nor clang > wouldn't warn about this by default... > Is this a new clang? It was clang-13, not sure if that made a difference. Arnd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 14:45 ` Marco Elver 2021-05-14 15:28 ` Arnd Bergmann @ 2021-05-14 18:40 ` Nathan Chancellor 1 sibling, 0 replies; 17+ messages in thread From: Nathan Chancellor @ 2021-05-14 18:40 UTC (permalink / raw) To: Marco Elver, Greg Kroah-Hartman, Paul E. McKenney Cc: Arnd Bergmann, Nick Desaulniers, Arnd Bergmann, Dmitry Vyukov, kasan-dev, LKML, clang-built-linux On 5/14/2021 7:45 AM, Marco Elver wrote: > On Fri, 14 May 2021 at 16:10, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: >> On Fri, May 14, 2021 at 04:00:08PM +0200, Arnd Bergmann wrote: >>> From: Arnd Bergmann <arnd@arndb.de> >>> >>> clang points out that an initcall funciton should return an 'int': >>> >>> kernel/kcsan/debugfs.c:274:15: error: returning 'void' from a function with incompatible result type 'int' >>> late_initcall(kcsan_debugfs_init); >>> ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ >>> include/linux/init.h:292:46: note: expanded from macro 'late_initcall' >>> #define late_initcall(fn) __define_initcall(fn, 7) >>> >>> Fixes: e36299efe7d7 ("kcsan, debugfs: Move debugfs file creation out of early init") >>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > [...] >>> >> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Reviewed-by: Marco Elver <elver@google.com> > > Thanks for catching this -- it boggles my mind why gcc nor clang > wouldn't warn about this by default... > Is this a new clang? KCSAN appears to only support x86_64, which also selects HAVE_ARCH_PREL32_RELOCATIONS, meaning that the initcalls never have their types validated because there is no assignment: https://elixir.bootlin.com/linux/v5.12.4/source/include/linux/init.h#L240 In the case of CONFIG_LTO_CLANG, the initcall function is called in the stub function, resulting in the error that we see here. Hopefully that makes sense :) Cheers, Nathan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 14:00 [PATCH] kcsan: fix debugfs initcall return type Arnd Bergmann 2021-05-14 14:10 ` Greg Kroah-Hartman @ 2021-05-14 18:29 ` Nathan Chancellor 2021-05-14 19:36 ` Paul E. McKenney 1 sibling, 1 reply; 17+ messages in thread From: Nathan Chancellor @ 2021-05-14 18:29 UTC (permalink / raw) To: Arnd Bergmann, Marco Elver, Nick Desaulniers, Paul E. McKenney, Greg Kroah-Hartman Cc: Arnd Bergmann, Dmitry Vyukov, kasan-dev, linux-kernel, clang-built-linux On 5/14/2021 7:00 AM, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > clang points out that an initcall funciton should return an 'int': > > kernel/kcsan/debugfs.c:274:15: error: returning 'void' from a function with incompatible result type 'int' > late_initcall(kcsan_debugfs_init); > ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ > include/linux/init.h:292:46: note: expanded from macro 'late_initcall' > #define late_initcall(fn) __define_initcall(fn, 7) > > Fixes: e36299efe7d7 ("kcsan, debugfs: Move debugfs file creation out of early init") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> For the record, this requires CONFIG_LTO_CLANG to be visible. Reviewed-by: Nathan Chancellor <nathan@kernel.org> > --- > kernel/kcsan/debugfs.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c > index c1dd02f3be8b..e65de172ccf7 100644 > --- a/kernel/kcsan/debugfs.c > +++ b/kernel/kcsan/debugfs.c > @@ -266,9 +266,10 @@ static const struct file_operations debugfs_ops = > .release = single_release > }; > > -static void __init kcsan_debugfs_init(void) > +static int __init kcsan_debugfs_init(void) > { > debugfs_create_file("kcsan", 0644, NULL, NULL, &debugfs_ops); > + return 0; > } > > late_initcall(kcsan_debugfs_init); > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 18:29 ` Nathan Chancellor @ 2021-05-14 19:36 ` Paul E. McKenney 2021-05-14 20:11 ` Nathan Chancellor 0 siblings, 1 reply; 17+ messages in thread From: Paul E. McKenney @ 2021-05-14 19:36 UTC (permalink / raw) To: Nathan Chancellor Cc: Arnd Bergmann, Marco Elver, Nick Desaulniers, Greg Kroah-Hartman, Arnd Bergmann, Dmitry Vyukov, kasan-dev, linux-kernel, clang-built-linux On Fri, May 14, 2021 at 11:29:18AM -0700, Nathan Chancellor wrote: > On 5/14/2021 7:00 AM, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > > > clang points out that an initcall funciton should return an 'int': > > > > kernel/kcsan/debugfs.c:274:15: error: returning 'void' from a function with incompatible result type 'int' > > late_initcall(kcsan_debugfs_init); > > ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ > > include/linux/init.h:292:46: note: expanded from macro 'late_initcall' > > #define late_initcall(fn) __define_initcall(fn, 7) > > > > Fixes: e36299efe7d7 ("kcsan, debugfs: Move debugfs file creation out of early init") > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > For the record, this requires CONFIG_LTO_CLANG to be visible. > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> Queued with the three Reviewed-by tags, thank you all! Nathan, I lost the thread on exactly what it is that requires that CONFIG_LTO_CLANG be visible. A naive reader might conclude that the compiler diagnostic does not appear unless CONFIG_LTO_CLANG=y, but that would be surprising (and yes, I have been surprised many times). If you are suggesting that the commit log be upgraded, could you please supply suggested wording? Once this is nailed down (or by Wednesday if I hear no more), I will rebase it to the bottom of the current kcsan stack, let it soak in -next for a couple of days, then send to Linus as a fix for a regression. Hopefully some time next week. Thanx, Paul > > --- > > kernel/kcsan/debugfs.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c > > index c1dd02f3be8b..e65de172ccf7 100644 > > --- a/kernel/kcsan/debugfs.c > > +++ b/kernel/kcsan/debugfs.c > > @@ -266,9 +266,10 @@ static const struct file_operations debugfs_ops = > > .release = single_release > > }; > > -static void __init kcsan_debugfs_init(void) > > +static int __init kcsan_debugfs_init(void) > > { > > debugfs_create_file("kcsan", 0644, NULL, NULL, &debugfs_ops); > > + return 0; > > } > > late_initcall(kcsan_debugfs_init); > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 19:36 ` Paul E. McKenney @ 2021-05-14 20:11 ` Nathan Chancellor 2021-05-14 20:18 ` Paul E. McKenney 0 siblings, 1 reply; 17+ messages in thread From: Nathan Chancellor @ 2021-05-14 20:11 UTC (permalink / raw) To: paulmck Cc: Arnd Bergmann, Marco Elver, Nick Desaulniers, Greg Kroah-Hartman, Arnd Bergmann, Dmitry Vyukov, kasan-dev, linux-kernel, clang-built-linux Hi Paul, On 5/14/2021 12:36 PM, Paul E. McKenney wrote: > On Fri, May 14, 2021 at 11:29:18AM -0700, Nathan Chancellor wrote: >> On 5/14/2021 7:00 AM, Arnd Bergmann wrote: >>> From: Arnd Bergmann <arnd@arndb.de> >>> >>> clang points out that an initcall funciton should return an 'int': >>> >>> kernel/kcsan/debugfs.c:274:15: error: returning 'void' from a function with incompatible result type 'int' >>> late_initcall(kcsan_debugfs_init); >>> ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ >>> include/linux/init.h:292:46: note: expanded from macro 'late_initcall' >>> #define late_initcall(fn) __define_initcall(fn, 7) >>> >>> Fixes: e36299efe7d7 ("kcsan, debugfs: Move debugfs file creation out of early init") >>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> >> For the record, this requires CONFIG_LTO_CLANG to be visible. >> >> Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > Queued with the three Reviewed-by tags, thank you all! > > Nathan, I lost the thread on exactly what it is that requires that > CONFIG_LTO_CLANG be visible. A naive reader might conclude that the > compiler diagnostic does not appear unless CONFIG_LTO_CLANG=y, but > that would be surprising (and yes, I have been surprised many times). > If you are suggesting that the commit log be upgraded, could you please > supply suggested wording? You can see my response to Marco here: https://lore.kernel.org/r/ad7fa126-f371-5a24-1d80-27fe8f655b05@kernel.org/ Maybe some improved wording might look like clang with CONFIG_LTO_CLANG points out that an initcall function should return an 'int' due to the changes made to the initcall macros in commit 3578ad11f3fb ("init: lto: fix PREL32 relocations"): ... Arnd, do you have any objections? Cheers, Nathan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 20:11 ` Nathan Chancellor @ 2021-05-14 20:18 ` Paul E. McKenney 2021-05-14 21:16 ` Arnd Bergmann 0 siblings, 1 reply; 17+ messages in thread From: Paul E. McKenney @ 2021-05-14 20:18 UTC (permalink / raw) To: Nathan Chancellor Cc: Arnd Bergmann, Marco Elver, Nick Desaulniers, Greg Kroah-Hartman, Arnd Bergmann, Dmitry Vyukov, kasan-dev, linux-kernel, clang-built-linux On Fri, May 14, 2021 at 01:11:05PM -0700, Nathan Chancellor wrote: > Hi Paul, > > On 5/14/2021 12:36 PM, Paul E. McKenney wrote: > > On Fri, May 14, 2021 at 11:29:18AM -0700, Nathan Chancellor wrote: > > > On 5/14/2021 7:00 AM, Arnd Bergmann wrote: > > > > From: Arnd Bergmann <arnd@arndb.de> > > > > > > > > clang points out that an initcall funciton should return an 'int': > > > > > > > > kernel/kcsan/debugfs.c:274:15: error: returning 'void' from a function with incompatible result type 'int' > > > > late_initcall(kcsan_debugfs_init); > > > > ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ > > > > include/linux/init.h:292:46: note: expanded from macro 'late_initcall' > > > > #define late_initcall(fn) __define_initcall(fn, 7) > > > > > > > > Fixes: e36299efe7d7 ("kcsan, debugfs: Move debugfs file creation out of early init") > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > > > > For the record, this requires CONFIG_LTO_CLANG to be visible. > > > > > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > > > Queued with the three Reviewed-by tags, thank you all! > > > > Nathan, I lost the thread on exactly what it is that requires that > > CONFIG_LTO_CLANG be visible. A naive reader might conclude that the > > compiler diagnostic does not appear unless CONFIG_LTO_CLANG=y, but > > that would be surprising (and yes, I have been surprised many times). > > If you are suggesting that the commit log be upgraded, could you please > > supply suggested wording? > > You can see my response to Marco here: > > https://lore.kernel.org/r/ad7fa126-f371-5a24-1d80-27fe8f655b05@kernel.org/ > > Maybe some improved wording might look like > > clang with CONFIG_LTO_CLANG points out that an initcall function should > return an 'int' due to the changes made to the initcall macros in commit > 3578ad11f3fb ("init: lto: fix PREL32 relocations"): OK, so the naive reading was correct, thank you! > ... > > Arnd, do you have any objections? In the meantime, here is what I have. Please let me know of any needed updates. Thanx, Paul ------------------------------------------------------------------------ commit fe1f4e1b099797d06bd8c66681eed4024c3cad67 Author: Arnd Bergmann <arnd@arndb.de> Date: Fri May 14 16:00:08 2021 +0200 kcsan: Fix debugfs initcall return type clang with CONFIG_LTO_CLANG points out that an initcall function should return an 'int' due to the changes made to the initcall macros in commit 3578ad11f3fb ("init: lto: fix PREL32 relocations"): kernel/kcsan/debugfs.c:274:15: error: returning 'void' from a function with incompatible result type 'int' late_initcall(kcsan_debugfs_init); ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ include/linux/init.h:292:46: note: expanded from macro 'late_initcall' #define late_initcall(fn) __define_initcall(fn, 7) Fixes: e36299efe7d7 ("kcsan, debugfs: Move debugfs file creation out of early init") Cc: stable <stable@vger.kernel.org> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reviewed-by: Marco Elver <elver@google.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Paul E. McKenney <paulmck@kernel.org> diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c index c1dd02f3be8b..e65de172ccf7 100644 --- a/kernel/kcsan/debugfs.c +++ b/kernel/kcsan/debugfs.c @@ -266,9 +266,10 @@ static const struct file_operations debugfs_ops = .release = single_release }; -static void __init kcsan_debugfs_init(void) +static int __init kcsan_debugfs_init(void) { debugfs_create_file("kcsan", 0644, NULL, NULL, &debugfs_ops); + return 0; } late_initcall(kcsan_debugfs_init); ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 20:18 ` Paul E. McKenney @ 2021-05-14 21:16 ` Arnd Bergmann 2021-05-14 23:01 ` Marco Elver 0 siblings, 1 reply; 17+ messages in thread From: Arnd Bergmann @ 2021-05-14 21:16 UTC (permalink / raw) To: Paul E. McKenney Cc: Nathan Chancellor, Marco Elver, Nick Desaulniers, Greg Kroah-Hartman, Dmitry Vyukov, kasan-dev, Linux Kernel Mailing List, clang-built-linux On Fri, May 14, 2021 at 10:18 PM Paul E. McKenney <paulmck@kernel.org> wrote: > On Fri, May 14, 2021 at 01:11:05PM -0700, Nathan Chancellor wrote: > > You can see my response to Marco here: > > > > https://lore.kernel.org/r/ad7fa126-f371-5a24-1d80-27fe8f655b05@kernel.org/ > > > > Maybe some improved wording might look like > > > > clang with CONFIG_LTO_CLANG points out that an initcall function should > > return an 'int' due to the changes made to the initcall macros in commit > > 3578ad11f3fb ("init: lto: fix PREL32 relocations"): > > OK, so the naive reading was correct, thank you! > > > ... > > > > Arnd, do you have any objections? > > In the meantime, here is what I have. Please let me know of any needed > updates. > Looks good to me, thanks for the improvements! Arnd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 21:16 ` Arnd Bergmann @ 2021-05-14 23:01 ` Marco Elver 2021-05-15 0:55 ` Paul E. McKenney ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Marco Elver @ 2021-05-14 23:01 UTC (permalink / raw) To: Arnd Bergmann Cc: Paul E. McKenney, Nathan Chancellor, Nick Desaulniers, Greg Kroah-Hartman, Dmitry Vyukov, kasan-dev, Linux Kernel Mailing List, clang-built-linux On Fri, May 14, 2021 at 11:16PM +0200, Arnd Bergmann wrote: > On Fri, May 14, 2021 at 10:18 PM Paul E. McKenney <paulmck@kernel.org> wrote: > > On Fri, May 14, 2021 at 01:11:05PM -0700, Nathan Chancellor wrote: > > > > You can see my response to Marco here: > > > > > > https://lore.kernel.org/r/ad7fa126-f371-5a24-1d80-27fe8f655b05@kernel.org/ > > > > > > Maybe some improved wording might look like > > > > > > clang with CONFIG_LTO_CLANG points out that an initcall function should > > > return an 'int' due to the changes made to the initcall macros in commit > > > 3578ad11f3fb ("init: lto: fix PREL32 relocations"): > > > > OK, so the naive reading was correct, thank you! > > > > > ... > > > > > > Arnd, do you have any objections? > > > > In the meantime, here is what I have. Please let me know of any needed > > updates. > > > > Looks good to me, thanks for the improvements! FWIW, this prompted me to see if I can convince the compiler to complain in all configs. The below is what I came up with and will send once the fix here has landed. Need to check a few other config+arch combinations (allyesconfig with gcc on x86_64 is good). Thanks, -- Marco ------ >8 ------ From 96c1c4e9902e96485268909d5ea8f91b9595e187 Mon Sep 17 00:00:00 2001 From: Marco Elver <elver@google.com> Date: Fri, 14 May 2021 21:08:50 +0200 Subject: [PATCH] init: verify that function is initcall_t at compile-time In the spirit of making it hard to misuse an interface, add a compile-time assertion in the CONFIG_HAVE_ARCH_PREL32_RELOCATIONS case to verify the initcall function matches initcall_t, because the inline asm bypasses any type-checking the compiler would otherwise do. This will help developers catch incorrect API use in all configurations. A recent example of this is: https://lkml.kernel.org/r/20210514140015.2944744-1-arnd@kernel.org Signed-off-by: Marco Elver <elver@google.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Joe Perches <joe@perches.com> Cc: Masahiro Yamada <masahiroy@kernel.org> Cc: Miguel Ojeda <ojeda@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: "Paul E. McKenney" <paulmck@kernel.org> --- include/linux/init.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/init.h b/include/linux/init.h index 045ad1650ed1..d82b4b2e1d25 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -242,7 +242,8 @@ extern bool initcall_debug; asm(".section \"" __sec "\", \"a\" \n" \ __stringify(__name) ": \n" \ ".long " __stringify(__stub) " - . \n" \ - ".previous \n"); + ".previous \n"); \ + static_assert(__same_type(initcall_t, &fn)); #else #define ____define_initcall(fn, __unused, __name, __sec) \ static initcall_t __name __used \ -- 2.31.1.751.gd2f1c929bd-goog ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 23:01 ` Marco Elver @ 2021-05-15 0:55 ` Paul E. McKenney 2021-05-18 23:20 ` Paul E. McKenney 2021-05-15 14:19 ` Miguel Ojeda 2021-05-16 5:17 ` Nathan Chancellor 2 siblings, 1 reply; 17+ messages in thread From: Paul E. McKenney @ 2021-05-15 0:55 UTC (permalink / raw) To: Marco Elver Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Greg Kroah-Hartman, Dmitry Vyukov, kasan-dev, Linux Kernel Mailing List, clang-built-linux On Sat, May 15, 2021 at 01:01:31AM +0200, Marco Elver wrote: > On Fri, May 14, 2021 at 11:16PM +0200, Arnd Bergmann wrote: > > On Fri, May 14, 2021 at 10:18 PM Paul E. McKenney <paulmck@kernel.org> wrote: > > > On Fri, May 14, 2021 at 01:11:05PM -0700, Nathan Chancellor wrote: > > > > > > You can see my response to Marco here: > > > > > > > > https://lore.kernel.org/r/ad7fa126-f371-5a24-1d80-27fe8f655b05@kernel.org/ > > > > > > > > Maybe some improved wording might look like > > > > > > > > clang with CONFIG_LTO_CLANG points out that an initcall function should > > > > return an 'int' due to the changes made to the initcall macros in commit > > > > 3578ad11f3fb ("init: lto: fix PREL32 relocations"): > > > > > > OK, so the naive reading was correct, thank you! > > > > > > > ... > > > > > > > > Arnd, do you have any objections? > > > > > > In the meantime, here is what I have. Please let me know of any needed > > > updates. > > > > > > > Looks good to me, thanks for the improvements! > > FWIW, this prompted me to see if I can convince the compiler to complain > in all configs. The below is what I came up with and will send once the > fix here has landed. Need to check a few other config+arch combinations > (allyesconfig with gcc on x86_64 is good). Cool! If I have not sent the pull request for Arnd's fix by Wednesday, please remind me. Thanx, Paul > Thanks, > -- Marco > > ------ >8 ------ > > >From 96c1c4e9902e96485268909d5ea8f91b9595e187 Mon Sep 17 00:00:00 2001 > From: Marco Elver <elver@google.com> > Date: Fri, 14 May 2021 21:08:50 +0200 > Subject: [PATCH] init: verify that function is initcall_t at compile-time > > In the spirit of making it hard to misuse an interface, add a > compile-time assertion in the CONFIG_HAVE_ARCH_PREL32_RELOCATIONS case > to verify the initcall function matches initcall_t, because the inline > asm bypasses any type-checking the compiler would otherwise do. This > will help developers catch incorrect API use in all configurations. > > A recent example of this is: > https://lkml.kernel.org/r/20210514140015.2944744-1-arnd@kernel.org > > Signed-off-by: Marco Elver <elver@google.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Joe Perches <joe@perches.com> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Cc: Miguel Ojeda <ojeda@kernel.org> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: "Paul E. McKenney" <paulmck@kernel.org> > --- > include/linux/init.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/init.h b/include/linux/init.h > index 045ad1650ed1..d82b4b2e1d25 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -242,7 +242,8 @@ extern bool initcall_debug; > asm(".section \"" __sec "\", \"a\" \n" \ > __stringify(__name) ": \n" \ > ".long " __stringify(__stub) " - . \n" \ > - ".previous \n"); > + ".previous \n"); \ > + static_assert(__same_type(initcall_t, &fn)); > #else > #define ____define_initcall(fn, __unused, __name, __sec) \ > static initcall_t __name __used \ > -- > 2.31.1.751.gd2f1c929bd-goog > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-15 0:55 ` Paul E. McKenney @ 2021-05-18 23:20 ` Paul E. McKenney 2021-05-19 8:09 ` Marco Elver 0 siblings, 1 reply; 17+ messages in thread From: Paul E. McKenney @ 2021-05-18 23:20 UTC (permalink / raw) To: Marco Elver Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Greg Kroah-Hartman, Dmitry Vyukov, kasan-dev, Linux Kernel Mailing List, clang-built-linux On Fri, May 14, 2021 at 05:55:50PM -0700, Paul E. McKenney wrote: > On Sat, May 15, 2021 at 01:01:31AM +0200, Marco Elver wrote: > > On Fri, May 14, 2021 at 11:16PM +0200, Arnd Bergmann wrote: > > > On Fri, May 14, 2021 at 10:18 PM Paul E. McKenney <paulmck@kernel.org> wrote: > > > > On Fri, May 14, 2021 at 01:11:05PM -0700, Nathan Chancellor wrote: > > > > > > > > You can see my response to Marco here: > > > > > > > > > > https://lore.kernel.org/r/ad7fa126-f371-5a24-1d80-27fe8f655b05@kernel.org/ > > > > > > > > > > Maybe some improved wording might look like > > > > > > > > > > clang with CONFIG_LTO_CLANG points out that an initcall function should > > > > > return an 'int' due to the changes made to the initcall macros in commit > > > > > 3578ad11f3fb ("init: lto: fix PREL32 relocations"): > > > > > > > > OK, so the naive reading was correct, thank you! > > > > > > > > > ... > > > > > > > > > > Arnd, do you have any objections? > > > > > > > > In the meantime, here is what I have. Please let me know of any needed > > > > updates. > > > > > > > > > > Looks good to me, thanks for the improvements! > > > > FWIW, this prompted me to see if I can convince the compiler to complain > > in all configs. The below is what I came up with and will send once the > > fix here has landed. Need to check a few other config+arch combinations > > (allyesconfig with gcc on x86_64 is good). > > Cool! > > If I have not sent the pull request for Arnd's fix by Wednesday, please > remind me. Except that I was slow getting Miguel Ojeda's Reviewed-by applied. I need to wait for -next to incorporate this change (hopefully by tomorrow, Pacific Time), and then test this. With luck, I will send this Thursday, Pacific Time. Thanx, Paul ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-18 23:20 ` Paul E. McKenney @ 2021-05-19 8:09 ` Marco Elver 0 siblings, 0 replies; 17+ messages in thread From: Marco Elver @ 2021-05-19 8:09 UTC (permalink / raw) To: Paul E. McKenney Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Greg Kroah-Hartman, Dmitry Vyukov, kasan-dev, Linux Kernel Mailing List, clang-built-linux On Wed, 19 May 2021 at 01:20, Paul E. McKenney <paulmck@kernel.org> wrote: [...] > > If I have not sent the pull request for Arnd's fix by Wednesday, please > > remind me. > > Except that I was slow getting Miguel Ojeda's Reviewed-by applied. > I need to wait for -next to incorporate this change (hopefully by > tomorrow, Pacific Time), and then test this. With luck, I will send > this Thursday, Pacific Time. Thank you! If I don't see anything by end of this week, I'll do the reminder then. Thanks, -- Marco ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 23:01 ` Marco Elver 2021-05-15 0:55 ` Paul E. McKenney @ 2021-05-15 14:19 ` Miguel Ojeda 2021-05-17 18:24 ` Paul E. McKenney 2021-05-16 5:17 ` Nathan Chancellor 2 siblings, 1 reply; 17+ messages in thread From: Miguel Ojeda @ 2021-05-15 14:19 UTC (permalink / raw) To: Marco Elver Cc: Arnd Bergmann, Paul E. McKenney, Nathan Chancellor, Nick Desaulniers, Greg Kroah-Hartman, Dmitry Vyukov, kasan-dev, Linux Kernel Mailing List, clang-built-linux On Sat, May 15, 2021 at 1:01 AM 'Marco Elver' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > FWIW, this prompted me to see if I can convince the compiler to complain > in all configs. The below is what I came up with and will send once the > fix here has landed. Need to check a few other config+arch combinations > (allyesconfig with gcc on x86_64 is good). +1 Works for LLVM=1 too (x86_64, small config). Reviewed-by: Miguel Ojeda <ojeda@kernel.org> Cheers, Miguel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-15 14:19 ` Miguel Ojeda @ 2021-05-17 18:24 ` Paul E. McKenney 0 siblings, 0 replies; 17+ messages in thread From: Paul E. McKenney @ 2021-05-17 18:24 UTC (permalink / raw) To: Miguel Ojeda Cc: Marco Elver, Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Greg Kroah-Hartman, Dmitry Vyukov, kasan-dev, Linux Kernel Mailing List, clang-built-linux On Sat, May 15, 2021 at 04:19:45PM +0200, Miguel Ojeda wrote: > On Sat, May 15, 2021 at 1:01 AM 'Marco Elver' via Clang Built Linux > <clang-built-linux@googlegroups.com> wrote: > > > > FWIW, this prompted me to see if I can convince the compiler to complain > > in all configs. The below is what I came up with and will send once the > > fix here has landed. Need to check a few other config+arch combinations > > (allyesconfig with gcc on x86_64 is good). > > +1 Works for LLVM=1 too (x86_64, small config). > > Reviewed-by: Miguel Ojeda <ojeda@kernel.org> I will applyon the next rebase, thank you! Thanx, Paul ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] kcsan: fix debugfs initcall return type 2021-05-14 23:01 ` Marco Elver 2021-05-15 0:55 ` Paul E. McKenney 2021-05-15 14:19 ` Miguel Ojeda @ 2021-05-16 5:17 ` Nathan Chancellor 2 siblings, 0 replies; 17+ messages in thread From: Nathan Chancellor @ 2021-05-16 5:17 UTC (permalink / raw) To: Marco Elver Cc: Arnd Bergmann, Paul E. McKenney, Nick Desaulniers, Greg Kroah-Hartman, Dmitry Vyukov, kasan-dev, Linux Kernel Mailing List, clang-built-linux Hi Marco, On Sat, May 15, 2021 at 01:01:31AM +0200, Marco Elver wrote: > FWIW, this prompted me to see if I can convince the compiler to complain > in all configs. The below is what I came up with and will send once the > fix here has landed. Need to check a few other config+arch combinations > (allyesconfig with gcc on x86_64 is good). > > Thanks, > -- Marco > > ------ >8 ------ > > >From 96c1c4e9902e96485268909d5ea8f91b9595e187 Mon Sep 17 00:00:00 2001 > From: Marco Elver <elver@google.com> > Date: Fri, 14 May 2021 21:08:50 +0200 > Subject: [PATCH] init: verify that function is initcall_t at compile-time > > In the spirit of making it hard to misuse an interface, add a > compile-time assertion in the CONFIG_HAVE_ARCH_PREL32_RELOCATIONS case > to verify the initcall function matches initcall_t, because the inline > asm bypasses any type-checking the compiler would otherwise do. This > will help developers catch incorrect API use in all configurations. > > A recent example of this is: > https://lkml.kernel.org/r/20210514140015.2944744-1-arnd@kernel.org > > Signed-off-by: Marco Elver <elver@google.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Joe Perches <joe@perches.com> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Cc: Miguel Ojeda <ojeda@kernel.org> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: "Paul E. McKenney" <paulmck@kernel.org> Hi Marco, I verified that I see an error without Arnd's patch with all supported KCSAN compilers when I apply this patch. clang-11: https://builds.tuxbuild.com/1sYcyUZoCS7hFS3qZMZsJgsA5bp/build.log clang-12: https://builds.tuxbuild.com/1sYcyRDtvvkaQQbGX435X8FUb6o/build.log clang-13: https://builds.tuxbuild.com/1sYcyPubVREo7Dl05zCKRRNh6RB/build.log gcc-11 had to be done locally as TuxSuite appears not to support gcc-11 so no nifty link: In file included from /home/nathan/cbl/src/korg-linux/include/asm-generic/atomic-instrumented.h:20, from /home/nathan/cbl/src/korg-linux/include/linux/atomic.h:82, from /home/nathan/cbl/src/korg-linux/kernel/kcsan/debugfs.c:10: /home/nathan/cbl/src/korg-linux/include/linux/build_bug.h:78:41: error: static assertion failed: "__same_type(initcall_t, &kcsan_debugfs_init)" 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~~~~~~~~~~~ /home/nathan/cbl/src/korg-linux/include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert' 77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr) | ^~~~~~~~~~~~~~~ /home/nathan/cbl/src/korg-linux/include/linux/init.h:246:9: note: in expansion of macro 'static_assert' 246 | static_assert(__same_type(initcall_t, &fn)); | ^~~~~~~~~~~~~ /home/nathan/cbl/src/korg-linux/include/linux/init.h:254:9: note: in expansion of macro '____define_initcall' 254 | ____define_initcall(fn, \ | ^~~~~~~~~~~~~~~~~~~ /home/nathan/cbl/src/korg-linux/include/linux/init.h:260:9: note: in expansion of macro '__unique_initcall' 260 | __unique_initcall(fn, id, __sec, __initcall_id(fn)) | ^~~~~~~~~~~~~~~~~ /home/nathan/cbl/src/korg-linux/include/linux/init.h:262:35: note: in expansion of macro '___define_initcall' 262 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id) | ^~~~~~~~~~~~~~~~~~ /home/nathan/cbl/src/korg-linux/include/linux/init.h:293:41: note: in expansion of macro '__define_initcall' 293 | #define late_initcall(fn) __define_initcall(fn, 7) | ^~~~~~~~~~~~~~~~~ /home/nathan/cbl/src/korg-linux/kernel/kcsan/debugfs.c:274:1: note: in expansion of macro 'late_initcall' 274 | late_initcall(kcsan_debugfs_init); | ^~~~~~~~~~~~~ make[3]: *** [/home/nathan/cbl/src/korg-linux/scripts/Makefile.build:273: kernel/kcsan/debugfs.o] Error 1 I did a series of builds against next-20210514 with gcc 8 through 10 and clang 11 through 13 targeting arm, arm64, i386, powerpc, s390, and x86_64 defconfig and allmodconfig with no errors with this patch on top of Arnd's. Repo and TuxSuite configuration below in case anyone cares :) https://git.kernel.org/pub/scm/linux/kernel/git/nathan/linux.git/log/?h=tuxsuite/initcall-static-assert https://gist.github.com/nathanchance/eb71e1c2287561a0de79ef28c3c521384 When you formally send it, please feel free to add: Reviewed-by: Nathan Chancellor <nathan@kernel.org> Tested-by: Nathan Chancellor <nathan@kernel.org> Cheers, Nathan > --- > include/linux/init.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/init.h b/include/linux/init.h > index 045ad1650ed1..d82b4b2e1d25 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -242,7 +242,8 @@ extern bool initcall_debug; > asm(".section \"" __sec "\", \"a\" \n" \ > __stringify(__name) ": \n" \ > ".long " __stringify(__stub) " - . \n" \ > - ".previous \n"); > + ".previous \n"); \ > + static_assert(__same_type(initcall_t, &fn)); > #else > #define ____define_initcall(fn, __unused, __name, __sec) \ > static initcall_t __name __used \ > -- > 2.31.1.751.gd2f1c929bd-goog > ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-05-19 8:10 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-14 14:00 [PATCH] kcsan: fix debugfs initcall return type Arnd Bergmann 2021-05-14 14:10 ` Greg Kroah-Hartman 2021-05-14 14:45 ` Marco Elver 2021-05-14 15:28 ` Arnd Bergmann 2021-05-14 18:40 ` Nathan Chancellor 2021-05-14 18:29 ` Nathan Chancellor 2021-05-14 19:36 ` Paul E. McKenney 2021-05-14 20:11 ` Nathan Chancellor 2021-05-14 20:18 ` Paul E. McKenney 2021-05-14 21:16 ` Arnd Bergmann 2021-05-14 23:01 ` Marco Elver 2021-05-15 0:55 ` Paul E. McKenney 2021-05-18 23:20 ` Paul E. McKenney 2021-05-19 8:09 ` Marco Elver 2021-05-15 14:19 ` Miguel Ojeda 2021-05-17 18:24 ` Paul E. McKenney 2021-05-16 5:17 ` Nathan Chancellor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).