From: Marco Elver <elver@google.com>
To: Arnd Bergmann <arnd@kernel.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>,
Nathan Chancellor <nathan@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Dmitry Vyukov <dvyukov@google.com>,
kasan-dev <kasan-dev@googlegroups.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
clang-built-linux <clang-built-linux@googlegroups.com>
Subject: Re: [PATCH] kcsan: fix debugfs initcall return type
Date: Sat, 15 May 2021 01:01:31 +0200 [thread overview]
Message-ID: <YJ8BS9fs5qrtQIzg@elver.google.com> (raw)
In-Reply-To: <CAK8P3a3O=DPgsXZpBxz+cPEHAzGaW+64GBDM4BMzAZQ+5w6Dow@mail.gmail.com>
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
next prev parent reply other threads:[~2021-05-14 23:01 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YJ8BS9fs5qrtQIzg@elver.google.com \
--to=elver@google.com \
--cc=arnd@kernel.org \
--cc=clang-built-linux@googlegroups.com \
--cc=dvyukov@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=kasan-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=paulmck@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox