diff mbox series

[RFC] um: implement CONFIG_CONSTRUCTORS for modules

Message ID 20200206204018.98f745aed1a1.I986f68ddedf1cb4ef59abf4fbcb5931ce99bc2e8@changeid
State Awaiting Upstream
Headers show
Series [RFC] um: implement CONFIG_CONSTRUCTORS for modules | expand

Commit Message

Johannes Berg Feb. 6, 2020, 7:40 p.m. UTC
From: Johannes Berg <johannes.berg@intel.com>

My previous attempt at implementing CONFIG_CONSTRUCTORS failed,
in part because it was badly done, but also because we just need
the gcc/libc constructors to run really early for what is really
a relatively normal userspace program. For some more details, see
commit 87c9366e1725 ("Revert "um: Enable CONFIG_CONSTRUCTORS"").

On the other hand, we *do* need constructors for modules, if we
want GCOV_KERNEL (and eventually KASAN) to work there, since the
constructors that gcc emits will need to be run.

So here's another attempt to implement CONFIG_CONSTRUCTORS: don't
touch the ones in the main binary, and don't do anything in the
main init code (do_ctors()), but still allow enabling the symbol
and associated code for modules.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 init/Kconfig        | 1 -
 init/main.c         | 8 ++++++--
 kernel/gcov/Kconfig | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

Comments

Richard Weinberger March 29, 2020, 7:31 p.m. UTC | #1
On Thu, Feb 6, 2020 at 8:40 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> From: Johannes Berg <johannes.berg@intel.com>
>
> My previous attempt at implementing CONFIG_CONSTRUCTORS failed,
> in part because it was badly done, but also because we just need
> the gcc/libc constructors to run really early for what is really
> a relatively normal userspace program. For some more details, see
> commit 87c9366e1725 ("Revert "um: Enable CONFIG_CONSTRUCTORS"").
>
> On the other hand, we *do* need constructors for modules, if we
> want GCOV_KERNEL (and eventually KASAN) to work there, since the
> constructors that gcc emits will need to be run.
>
> So here's another attempt to implement CONFIG_CONSTRUCTORS: don't
> touch the ones in the main binary, and don't do anything in the
> main init code (do_ctors()), but still allow enabling the symbol
> and associated code for modules.

I have to admit I don't know all details about constructors.
Do I understand this correctly that constructor code is never code which
depends on kernel internals? So it will always be gcc internal stuff?

If so, this approach will work and you can have my:
Acked-by: Richard Weinberger <richard@nod.at>
Johannes Berg March 29, 2020, 7:32 p.m. UTC | #2
On Sun, 2020-03-29 at 21:31 +0200, Richard Weinberger wrote:
> On Thu, Feb 6, 2020 at 8:40 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > My previous attempt at implementing CONFIG_CONSTRUCTORS failed,
> > in part because it was badly done, but also because we just need
> > the gcc/libc constructors to run really early for what is really
> > a relatively normal userspace program. For some more details, see
> > commit 87c9366e1725 ("Revert "um: Enable CONFIG_CONSTRUCTORS"").
> > 
> > On the other hand, we *do* need constructors for modules, if we
> > want GCOV_KERNEL (and eventually KASAN) to work there, since the
> > constructors that gcc emits will need to be run.
> > 
> > So here's another attempt to implement CONFIG_CONSTRUCTORS: don't
> > touch the ones in the main binary, and don't do anything in the
> > main init code (do_ctors()), but still allow enabling the symbol
> > and associated code for modules.
> 
> I have to admit I don't know all details about constructors.
> Do I understand this correctly that constructor code is never code which
> depends on kernel internals? So it will always be gcc internal stuff?

Yeah, more or less. You *can* mark something as
__attribute__((constructor)) and the compiler will emit it properly, but
that's not really something we do in the kernel.

johannes
Richard Weinberger March 29, 2020, 7:36 p.m. UTC | #3
On Sun, Mar 29, 2020 at 9:32 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Sun, 2020-03-29 at 21:31 +0200, Richard Weinberger wrote:
> > On Thu, Feb 6, 2020 at 8:40 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> > > From: Johannes Berg <johannes.berg@intel.com>
> > >
> > > My previous attempt at implementing CONFIG_CONSTRUCTORS failed,
> > > in part because it was badly done, but also because we just need
> > > the gcc/libc constructors to run really early for what is really
> > > a relatively normal userspace program. For some more details, see
> > > commit 87c9366e1725 ("Revert "um: Enable CONFIG_CONSTRUCTORS"").
> > >
> > > On the other hand, we *do* need constructors for modules, if we
> > > want GCOV_KERNEL (and eventually KASAN) to work there, since the
> > > constructors that gcc emits will need to be run.
> > >
> > > So here's another attempt to implement CONFIG_CONSTRUCTORS: don't
> > > touch the ones in the main binary, and don't do anything in the
> > > main init code (do_ctors()), but still allow enabling the symbol
> > > and associated code for modules.
> >
> > I have to admit I don't know all details about constructors.
> > Do I understand this correctly that constructor code is never code which
> > depends on kernel internals? So it will always be gcc internal stuff?
>
> Yeah, more or less. You *can* mark something as
> __attribute__((constructor)) and the compiler will emit it properly, but
> that's not really something we do in the kernel.

Yeah, I grepped for that gcc attribute and didn't find anything because first
I thought the main use cause of CONFIG_CONSTRUCTORS is supporting
such things.
As soon this is the case, UML will fail badly. But this can always happen. ;-)
Johannes Berg March 29, 2020, 7:37 p.m. UTC | #4
On Sun, 2020-03-29 at 21:36 +0200, Richard Weinberger wrote:
> On Sun, Mar 29, 2020 at 9:32 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Sun, 2020-03-29 at 21:31 +0200, Richard Weinberger wrote:
> > > On Thu, Feb 6, 2020 at 8:40 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> > > > From: Johannes Berg <johannes.berg@intel.com>
> > > > 
> > > > My previous attempt at implementing CONFIG_CONSTRUCTORS failed,
> > > > in part because it was badly done, but also because we just need
> > > > the gcc/libc constructors to run really early for what is really
> > > > a relatively normal userspace program. For some more details, see
> > > > commit 87c9366e1725 ("Revert "um: Enable CONFIG_CONSTRUCTORS"").
> > > > 
> > > > On the other hand, we *do* need constructors for modules, if we
> > > > want GCOV_KERNEL (and eventually KASAN) to work there, since the
> > > > constructors that gcc emits will need to be run.
> > > > 
> > > > So here's another attempt to implement CONFIG_CONSTRUCTORS: don't
> > > > touch the ones in the main binary, and don't do anything in the
> > > > main init code (do_ctors()), but still allow enabling the symbol
> > > > and associated code for modules.
> > > 
> > > I have to admit I don't know all details about constructors.
> > > Do I understand this correctly that constructor code is never code which
> > > depends on kernel internals? So it will always be gcc internal stuff?
> > 
> > Yeah, more or less. You *can* mark something as
> > __attribute__((constructor)) and the compiler will emit it properly, but
> > that's not really something we do in the kernel.
> 
> Yeah, I grepped for that gcc attribute and didn't find anything because first
> I thought the main use cause of CONFIG_CONSTRUCTORS is supporting
> such things.
> As soon this is the case, UML will fail badly. But this can always happen. ;-)

:)

I think even in the normal kernel many things will fail - it's pretty
early even there.

For now, I think this is fine... Maybe we should remove some of the gcov
hacks though? Haven't looked at that closely.

johannes
diff mbox series

Patch

diff --git a/init/Kconfig b/init/Kconfig
index 47d40f399000..a34064a031a5 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -54,7 +54,6 @@  config CC_DISABLE_WARN_MAYBE_UNINITIALIZED
 
 config CONSTRUCTORS
 	bool
-	depends on !UML
 
 config IRQ_WORK
 	bool
diff --git a/init/main.c b/init/main.c
index 2cd736059416..194d896ad651 100644
--- a/init/main.c
+++ b/init/main.c
@@ -784,10 +784,14 @@  asmlinkage __visible void __init start_kernel(void)
 	arch_call_rest_init();
 }
 
-/* Call all constructor functions linked into the kernel. */
+/*
+ * Call all constructor functions linked into the kernel, except for
+ * UML since that is a "normal" userspace program where they will be
+ * called automatically even before main().
+ */
 static void __init do_ctors(void)
 {
-#ifdef CONFIG_CONSTRUCTORS
+#if defined(CONFIG_CONSTRUCTORS) && !defined(CONFIG_UML)
 	ctor_fn_t *fn = (ctor_fn_t *) __ctors_start;
 
 	for (; fn < (ctor_fn_t *) __ctors_end; fn++)
diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index 3941a9c48f83..060e8e726755 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -4,7 +4,7 @@  menu "GCOV-based kernel profiling"
 config GCOV_KERNEL
 	bool "Enable gcov-based kernel profiling"
 	depends on DEBUG_FS
-	select CONSTRUCTORS if !UML
+	select CONSTRUCTORS
 	default n
 	---help---
 	This option enables gcov-based code profiling (e.g. for code coverage