Fix the absence of warning message and kernel tainting when initializing
drivers from the `drivers/staging` subtree from initcalls (when
configured as built-in).

When such a driver is built as module and the module is loaded, the
`load_module()` function taints the kernel to signal code of unknown
quality is loaded, and produces a warning like this:

[    8.076352] rts5208: module is from the staging directory, the
quality is unknown, you have been warned.

The same behaviour is absent, however, when a staging driver is compiled
as built-in on the kernel image, since loading it happens through
initcalls and not through load_module().

This might prevent relevant information of being available on a bug
report (i.e. on a panic log) among other possible problems.

NOTES:
- The patch is written in such a way that all non-staging drivers are
  kept the way they were, except for staging drivers built with
  `-DSTAGING_CODE`.
- Since it changes some macros related to clang LTO as well, I tested it
  and it works properly in kernels compiled with both clang and gcc.
- Some `checkpatch.pl` errors, warnings and checks (with `--strict`) are
  present. Some were already there, some I introduced but I think
  they're unavoidable. Some IMHO don´t make sense at all, I think they
  would apply for most regular macros but initcall macros are just way
  different.

Signed-off-by: Ágatha Isabelle Chris Moreira Guedes <code@agatha.dev>
---
CHANGELOG
v2:
- Changed the way we hook into the initcalls as suggested by Uwe, and
  moved the logic from `include/linux/module.h` to
  `include/linux/init.h`.
- Adjusted accordingly to work with both GCC and clang with
  `CONFIG_LTO_CLANG=y`, since some init scripts required that.
- Fixed the missing space pointed out by Dan & other minor string
  details.
- Changed the subject, since it became somewhat more of an init thing
  than a staging thing.

 drivers/staging/Makefile |  2 ++
 include/linux/init.h     | 76 ++++++++++++++++++++++++++++++++++------
 init/main.c              | 20 +++++++++++
 kernel/module/main.c     |  4 +--
 4 files changed, 88 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 5390879b5d1b..7cea13436426 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for staging directory
 
+subdir-ccflags-y += -DSTAGING_CODE
+
 obj-y				+= media/
 obj-$(CONFIG_FB_OLPC_DCON)	+= olpc_dcon/
 obj-$(CONFIG_RTL8192E)		+= rtl8192e/
diff --git a/include/linux/init.h b/include/linux/init.h
index 58cef4c2e59a..c7afee978b42 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -188,6 +188,8 @@ extern struct module __this_module;
 
 #ifndef __ASSEMBLY__
 
+extern void staging_taint(const char *code_id, bool module);
+
 /*
  * initcalls are now grouped by functionality into separate
  * subsections. Ordering inside the subsections is determined
@@ -220,6 +222,14 @@ extern struct module __this_module;
 	__PASTE(__,						\
 	__PASTE(__iid, id))))
 
+#define __define_stub_fn(___stub, fn, __taintcall)		\
+	int __init ___stub(void);				\
+	int __init ___stub(void)				\
+	{							\
+		__taintcall();					\
+		return fn();					\
+	}							\
+
 #ifdef CONFIG_LTO_CLANG
 /*
  * With LTO, the compiler doesn't necessarily obey link order for
@@ -230,31 +240,75 @@ extern struct module __this_module;
 #define __initcall_section(__sec, __iid)			\
 	#__sec ".init.." #__iid
 
+#define ___define_initcall_stub(__stub, fn, __taintcall)	\
+	__define_stub_fn(__stub, fn, __taintcall)		\
+	__ADDRESSABLE(__stub)
+#else
+#define __initcall_section(__sec, __iid)			\
+	#__sec ".init"
+
+#define ___define_initcall_stub(__stub, fn, __taintcall)	\
+	__ADDRESSABLE(fn)
+#endif /* CONFIG_LTO_CLANG */
+
+#ifdef STAGING_CODE
+
 /*
  * With LTO, the compiler can rename static functions to avoid
  * global naming collisions. We use a global stub function for
  * initcalls to create a stable symbol name whose address can be
  * taken in inline assembly when PREL32 relocations are used.
+ *
+ * Moreover, when there's staging code, regardless of LTO, we
+ * need to wrap the function inside a new one to taint the
+ * kernel and warn about it in the log. So we need this special
+ * symbol name for the wrapper regardless of that.
  */
 #define __initcall_stub(fn, __iid, id)				\
 	__initcall_name(initstub, __iid, id)
 
+#define __staging_taint_fn()					\
+	staging_taint(KBUILD_MODNAME, false)
+
+#ifdef CONFIG_LTO_CLANG
+
 #define __define_initcall_stub(__stub, fn)			\
-	int __init __stub(void);				\
-	int __init __stub(void)					\
-	{ 							\
-		return fn();					\
-	}							\
-	__ADDRESSABLE(__stub)
-#else
-#define __initcall_section(__sec, __iid)			\
-	#__sec ".init"
+	___define_initcall_stub(__stub, fn, __staging_taint_fn)
+
+#else /* no CONFIG_LTO_CLANG */
+
+#define __define_initcall_stub(_stub, fn)			\
+	__define_stub_fn(_stub, fn, __staging_taint_fn)		\
+	__ADDRESSABLE(_stub)
+
+#endif /* CONFIG_LTO_CLANG */
+
+#else /* no STAGING_CODE */
+
+#ifdef CONFIG_LTO_CLANG
+
+/*
+ * Same case as before, it's not staging code but there's LTO
+ */
+#define __initcall_stub(fn, __iid, id)				\
+	__initcall_name(initstub, __iid, id)
 
+#else /* no CONFIG_LTO_CLANG*/
+
+/* For no LTO outside staging code, the vast majority of drivers
+ * can just be built with their regular symbol names as they
+ * just have been built all the time
+ */
 #define __initcall_stub(fn, __iid, id)	fn
 
+#endif /* CONFIG_LTO_CLANG */
+
+#define __staging_taint_fn()
+
 #define __define_initcall_stub(__stub, fn)			\
-	__ADDRESSABLE(fn)
-#endif
+	___define_initcall_stub(__stub, fn, __staging_taint_fn)
+
+#endif /* STAGING_CODE */
 
 #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
 #define ____define_initcall(fn, __stub, __name, __sec)		\
diff --git a/init/main.c b/init/main.c
index 206acdde51f5..a297675dd397 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1602,3 +1602,23 @@ static noinline void __init kernel_init_freeable(void)
 
 	integrity_load_keys();
 }
+
+#ifdef CONFIG_STAGING
+/**
+ * staging_init_taint() - We need to taint the kernel whenever staging code
+ * is initialized (from built-in drivers) or loaded (as modules) and issue
+ * a warning the first time it happens.
+ */
+void staging_taint(const char *code_id, bool module)
+{
+	char *code_type = module ? "module" : "builtin driver";
+
+	pr_warn("%s %s: The kernel contains code from staging directory"
+		", the quality is unknown, you have been warned.\n",
+		code_type, code_id);
+
+	add_taint(TAINT_CRAP, LOCKDEP_STILL_OK);
+}
+EXPORT_SYMBOL(staging_taint);
+
+#endif /* CONFIG_STAGING */
diff --git a/kernel/module/main.c b/kernel/module/main.c
index d18a94b973e1..d7d33336ab43 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2039,9 +2039,7 @@ static void module_augment_kernel_taints(struct module *mod, struct load_info *i
 	check_modinfo_retpoline(mod, info);
 
 	if (get_modinfo(info, "staging")) {
-		add_taint_module(mod, TAINT_CRAP, LOCKDEP_STILL_OK);
-		pr_warn("%s: module is from the staging directory, the quality "
-			"is unknown, you have been warned.\n", mod->name);
+		staging_taint(mod->name, true);
 	}
 
 	if (is_livepatch_module(mod)) {
-- 
2.45.2

On Thu, Jul 04, 2024 at 09:20:49PM -0300, Ágatha Isabelle Chris Moreira Guedes wrote:
> +#ifdef CONFIG_STAGING
> +/**
> + * staging_init_taint() - We need to taint the kernel whenever staging code
> + * is initialized (from built-in drivers) or loaded (as modules) and issue
> + * a warning the first time it happens.
> + */
> +void staging_taint(const char *code_id, bool module)
> +{
> +	char *code_type = module ? "module" : "builtin driver";
> +
> +	pr_warn("%s %s: The kernel contains code from staging directory"
> +		", the quality is unknown, you have been warned.\n",

I wasn't going to say anything but since you're resending anyway,
please, don't break the line like this so it starts with a comma.  The
comma can go on the line before.  But if it couldn't then we would move
the whole word "directory," to the next line.

regards,
dan carpenter

Hello,

Please disregard this one, there will be a v3 soon.

There were some oversights on my side regarding some review points, and
also the code itself.

On Thu, Jul 04, 2024 at 09:20:49PM GMT, Ágatha Isabelle Chris Moreira Guedes wrote:
> diff --git a/init/main.c b/init/main.c
> index 206acdde51f5..a297675dd397 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1602,3 +1602,23 @@ static noinline void __init kernel_init_freeable(void)
>  
>  	integrity_load_keys();
>  }
> +
> +#ifdef CONFIG_STAGING

Here we break the building in kernel configs where `CONFIG_STAGING=n`
because:

> +/**
> + * staging_init_taint() - We need to taint the kernel whenever staging code
> + * is initialized (from built-in drivers) or loaded (as modules) and issue
> + * a warning the first time it happens.
> + */
> +void staging_taint(const char *code_id, bool module)
> +{

^ `staging_taint()` is built only with `CONFIG_STAGING`, and

> +#endif /* CONFIG_STAGING */
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index d18a94b973e1..d7d33336ab43 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2039,9 +2039,7 @@ static void module_augment_kernel_taints(struct module *mod, struct load_info *i
>  	check_modinfo_retpoline(mod, info);
>  
>  	if (get_modinfo(info, "staging")) {
> -		add_taint_module(mod, TAINT_CRAP, LOCKDEP_STILL_OK);

here we call `staging_taint()` regardless of having `CONFIG_STAGING` or
not. So on all kernels with modules enabled this patch as it is now
would break the builds.

I just noticed a couple of hours after submission when I was looking at
it again.

I'll fix it and submit v3 soon.