[PATCH] init/main.c: Do jump_label_init before early_security_init

[PATCH] init/main.c: Do jump_label_init before early_security_init

[KP Singh]

LSM indirect calls being are now replaced by static calls, this requires
a jumpt_table_init before early_security_init where LSM hooks and their
static calls and keys are initialized.

Fixes: 2732ad5ecd5b ("lsm: replace indirect LSM hook calls with static calls")
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 init/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/init/main.c b/init/main.c
index 206acdde51f5..5bd45af7a49e 100644
--- a/init/main.c
+++ b/init/main.c
@@ -922,6 +922,8 @@ void start_kernel(void)
 	boot_cpu_init();
 	page_address_init();
 	pr_notice("%s", linux_banner);
+	/* LSM and command line parameters use static keys */
+	jump_label_init();
 	early_security_init();
 	setup_arch(&command_line);
 	setup_boot_config();
@@ -933,8 +935,6 @@ void start_kernel(void)
 	boot_cpu_hotplug_init();
 
 	pr_notice("Kernel command line: %s\n", saved_command_line);
-	/* parameters may set static keys */
-	jump_label_init();
 	parse_early_param();
 	after_dashes = parse_args("Booting kernel",
 				  static_command_line, __start___param,
-- 
2.46.0.rc2.264.g509ed76dc8-goog

Re: [PATCH] init/main.c: Do jump_label_init before early_security_init

[Paul Moore]

On Wed, Jul 31, 2024 at 5:34 PM KP Singh <kpsingh@kernel.org> wrote:
>
> LSM indirect calls being are now replaced by static calls, this requires
> a jumpt_table_init before early_security_init where LSM hooks and their
> static calls and keys are initialized.
>
> Fixes: 2732ad5ecd5b ("lsm: replace indirect LSM hook calls with static calls")
> Signed-off-by: KP Singh <kpsingh@kernel.org>
> ---
>  init/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Does this look okay, static call folks?

> diff --git a/init/main.c b/init/main.c
> index 206acdde51f5..5bd45af7a49e 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -922,6 +922,8 @@ void start_kernel(void)
>         boot_cpu_init();
>         page_address_init();
>         pr_notice("%s", linux_banner);
> +       /* LSM and command line parameters use static keys */
> +       jump_label_init();
>         early_security_init();
>         setup_arch(&command_line);
>         setup_boot_config();
> @@ -933,8 +935,6 @@ void start_kernel(void)
>         boot_cpu_hotplug_init();
>
>         pr_notice("Kernel command line: %s\n", saved_command_line);
> -       /* parameters may set static keys */
> -       jump_label_init();
>         parse_early_param();
>         after_dashes = parse_args("Booting kernel",
>                                   static_command_line, __start___param,
> --
> 2.46.0.rc2.264.g509ed76dc8-goog

-- 
paul-moore.com

Re: [PATCH] init/main.c: Do jump_label_init before early_security_init

[Nathan Chancellor]

On Wed, Jul 31, 2024 at 09:15:04PM -0400, Paul Moore wrote:
> On Wed, Jul 31, 2024 at 5:34 PM KP Singh <kpsingh@kernel.org> wrote:
> >
> > LSM indirect calls being are now replaced by static calls, this requires
> > a jumpt_table_init before early_security_init where LSM hooks and their
> > static calls and keys are initialized.
> >
> > Fixes: 2732ad5ecd5b ("lsm: replace indirect LSM hook calls with static calls")
> > Signed-off-by: KP Singh <kpsingh@kernel.org>
> > ---
> >  init/main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Does this look okay, static call folks?

For the record, I tested this patch since I noticed the warnings like
Boris did and it appears to break booting for me with certain ARCH=arm
configurations in QEMU.

  $ cat arch/arm/configs/repro.config
  CONFIG_JUMP_LABEL=y
  CONFIG_SECURITY=y
  CONFIG_SECURITY_LOCKDOWN_LSM=y
  CONFIG_SECURITY_LOCKDOWN_LSM_EARLY=y

  $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- mrproper defconfig repro.config zImage

  $ qemu-system-arm \
      -display none \
      -nodefaults \
      -no-reboot \
      -machine virt \
      -append 'console=ttyAMA0 earlycon' \
      -kernel arch/arm/boot/zImage \
      -initrd rootfs.cpio \
      -m 512m \
      -serial mon:stdio
  <hangs with no output>

Without this patch, that same configuration works fine (with the warning
from before):

  [    0.000000] Booting Linux on physical CPU 0x0
  [    0.000000] Linux version 6.11.0-rc1-next-20240730 (nathan@m3-large-x86) (arm-linux-gnueabi-gcc (GCC) 14.1.0, GNU ld (GNU Binutils) 2.42) #1 SMP Thu Aug  1 05:44:11 UTC 2024
  [    0.000000] ------------[ cut here ]------------
  [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:199 static_key_enable_cpuslocked+0xb8/0xf4
  [    0.000000] static_key_enable_cpuslocked(): static key '0xc1fb4930' used before call to jump_label_init()
  [    0.000000] Modules linked in:
  [    0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.11.0-rc1-next-20240730 #1
  [    0.000000] Call trace:
  [    0.000000]  unwind_backtrace from show_stack+0x10/0x14
  [    0.000000]  show_stack from dump_stack_lvl+0x54/0x68
  [    0.000000]  dump_stack_lvl from __warn+0x80/0x114
  [    0.000000]  __warn from warn_slowpath_fmt+0x124/0x18c
  [    0.000000]  warn_slowpath_fmt from static_key_enable_cpuslocked+0xb8/0xf4
  [    0.000000]  static_key_enable_cpuslocked from static_key_enable+0x14/0x1c
  [    0.000000]  static_key_enable from security_add_hooks+0xc4/0xfc
  [    0.000000]  security_add_hooks from lockdown_lsm_init+0x18/0x24
  [    0.000000]  lockdown_lsm_init from initialize_lsm+0x44/0x7c
  [    0.000000]  initialize_lsm from early_security_init+0x44/0x50
  [    0.000000]  early_security_init from start_kernel+0x64/0x6bc
  [    0.000000]  start_kernel from 0x0
  [    0.000000] ---[ end trace 0000000000000000 ]---

I haven't tried to fire up GDB to figure out why it is exploding early
since it is late for me but I figured I would get the report out first.
The rootfs is available from [1] (arm-rootfs.cpio.zst, decompress it
with zstd first); it just shuts down the machine on boot.

Cheers,
Nathan

[1]: https://github.com/ClangBuiltLinux/boot-utils/releases/latest

> > diff --git a/init/main.c b/init/main.c
> > index 206acdde51f5..5bd45af7a49e 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -922,6 +922,8 @@ void start_kernel(void)
> >         boot_cpu_init();
> >         page_address_init();
> >         pr_notice("%s", linux_banner);
> > +       /* LSM and command line parameters use static keys */
> > +       jump_label_init();
> >         early_security_init();
> >         setup_arch(&command_line);
> >         setup_boot_config();
> > @@ -933,8 +935,6 @@ void start_kernel(void)
> >         boot_cpu_hotplug_init();
> >
> >         pr_notice("Kernel command line: %s\n", saved_command_line);
> > -       /* parameters may set static keys */
> > -       jump_label_init();
> >         parse_early_param();
> >         after_dashes = parse_args("Booting kernel",
> >                                   static_command_line, __start___param,
> > --
> > 2.46.0.rc2.264.g509ed76dc8-goog
> 
> -- 
> paul-moore.com

Re: [PATCH] init/main.c: Do jump_label_init before early_security_init

[Nathan Chancellor]

On Wed, Jul 31, 2024 at 10:48:06PM -0700, Nathan Chancellor wrote:
> On Wed, Jul 31, 2024 at 09:15:04PM -0400, Paul Moore wrote:
> > On Wed, Jul 31, 2024 at 5:34 PM KP Singh <kpsingh@kernel.org> wrote:
> > >
> > > LSM indirect calls being are now replaced by static calls, this requires
> > > a jumpt_table_init before early_security_init where LSM hooks and their
> > > static calls and keys are initialized.
> > >
> > > Fixes: 2732ad5ecd5b ("lsm: replace indirect LSM hook calls with static calls")
> > > Signed-off-by: KP Singh <kpsingh@kernel.org>
> > > ---
> > >  init/main.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > Does this look okay, static call folks?
> 
> For the record, I tested this patch since I noticed the warnings like
> Boris did and it appears to break booting for me with certain ARCH=arm
> configurations in QEMU.
> 
>   $ cat arch/arm/configs/repro.config
>   CONFIG_JUMP_LABEL=y
>   CONFIG_SECURITY=y
>   CONFIG_SECURITY_LOCKDOWN_LSM=y
>   CONFIG_SECURITY_LOCKDOWN_LSM_EARLY=y
> 
>   $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- mrproper defconfig repro.config zImage
> 
>   $ qemu-system-arm \
>       -display none \
>       -nodefaults \
>       -no-reboot \
>       -machine virt \
>       -append 'console=ttyAMA0 earlycon' \
>       -kernel arch/arm/boot/zImage \
>       -initrd rootfs.cpio \
>       -m 512m \
>       -serial mon:stdio
>   <hangs with no output>
> 
> Without this patch, that same configuration works fine (with the warning
> from before):
> 
>   [    0.000000] Booting Linux on physical CPU 0x0
>   [    0.000000] Linux version 6.11.0-rc1-next-20240730 (nathan@m3-large-x86) (arm-linux-gnueabi-gcc (GCC) 14.1.0, GNU ld (GNU Binutils) 2.42) #1 SMP Thu Aug  1 05:44:11 UTC 2024
>   [    0.000000] ------------[ cut here ]------------
>   [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:199 static_key_enable_cpuslocked+0xb8/0xf4
>   [    0.000000] static_key_enable_cpuslocked(): static key '0xc1fb4930' used before call to jump_label_init()
>   [    0.000000] Modules linked in:
>   [    0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.11.0-rc1-next-20240730 #1
>   [    0.000000] Call trace:
>   [    0.000000]  unwind_backtrace from show_stack+0x10/0x14
>   [    0.000000]  show_stack from dump_stack_lvl+0x54/0x68
>   [    0.000000]  dump_stack_lvl from __warn+0x80/0x114
>   [    0.000000]  __warn from warn_slowpath_fmt+0x124/0x18c
>   [    0.000000]  warn_slowpath_fmt from static_key_enable_cpuslocked+0xb8/0xf4
>   [    0.000000]  static_key_enable_cpuslocked from static_key_enable+0x14/0x1c
>   [    0.000000]  static_key_enable from security_add_hooks+0xc4/0xfc
>   [    0.000000]  security_add_hooks from lockdown_lsm_init+0x18/0x24
>   [    0.000000]  lockdown_lsm_init from initialize_lsm+0x44/0x7c
>   [    0.000000]  initialize_lsm from early_security_init+0x44/0x50
>   [    0.000000]  early_security_init from start_kernel+0x64/0x6bc
>   [    0.000000]  start_kernel from 0x0
>   [    0.000000] ---[ end trace 0000000000000000 ]---
> 
> I haven't tried to fire up GDB to figure out why it is exploding early
> since it is late for me but I figured I would get the report out first.
> The rootfs is available from [1] (arm-rootfs.cpio.zst, decompress it
> with zstd first); it just shuts down the machine on boot.
> 
> Cheers,
> Nathan
> 
> [1]: https://github.com/ClangBuiltLinux/boot-utils/releases/latest

Also, looking at my build logs, this patch does not appear to resolve
the static call warning I see with certain x86_64 distribution
configurations such as Fedora's (not sure if it was or not):

https://src.fedoraproject.org/rpms/kernel/raw/rawhide/f/kernel-x86_64-fedora.config

[    0.000000] Linux version 6.11.0-rc1-next-20240730-dirty (nathan@m3-large-x86) (x86_64-linux-gcc (GCC) 14.1.0, GNU ld (GNU Binutils) 2.42) #1 SMP PREEMPT_DYNAMIC Thu Aug  1 06:09:54 UTC 2024
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/static_call_inline.c:153 __static_call_update+0x18c/0x1f0
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.11.0-rc1-next-20240730-dirty #1
[    0.000000] RIP: 0010:__static_call_update+0x18c/0x1f0
[    0.000000] Code: 80 3d b6 7b 49 02 00 0f 85 7b ff ff ff 4c 89 f6 48 c7 c7 90 3b bc 8b c6 05 9f 7b 49 02 01 e8 2b 5c da ff 0f 0b e9 5e ff ff ff <0f> 0b 48 c7 c7 40 f2 5f 8c e8 36 72 e4 00 48 8b 44 24 28 65 48 2b
[    0.000000] RSP: 0000:ffffffff8c403e28 EFLAGS: 00010046 ORIG_RAX: 0000000000000000
[    0.000000] RAX: 0000000000000000 RBX: ffffffff8b19cd60 RCX: 000000005e199be9
[    0.000000] RDX: 0000000000000000 RSI: ffffffff8d302a70 RDI: ffffffff8c472500
[    0.000000] RBP: ffffffff8c6a01a0 R08: 00000000ff5e199b R09: fffffffffffbf82b
[    0.000000] R10: 0000000000000000 R11: 0000000000013f90 R12: ffffffff8b4d0cb0
[    0.000000] R13: 0000000000000001 R14: ffffffff8a77e700 R15: 00000000000147d0
[    0.000000] FS:  0000000000000000(0000) GS:ffffffff8ce3e000(0000) knlGS:0000000000000000
[    0.000000] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.000000] CR2: ffff8880000147d0 CR3: 000000000af46000 CR4: 00000000000000b0
[    0.000000] Call Trace:
[    0.000000]  <TASK>
[    0.000000]  ? __static_call_update+0x18c/0x1f0
[    0.000000]  ? __warn.cold+0x93/0xed
[    0.000000]  ? __static_call_update+0x18c/0x1f0
[    0.000000]  ? report_bug+0xff/0x140
[    0.000000]  ? __pfx_lockdown_is_locked_down+0x10/0x10
[    0.000000]  ? early_fixup_exception+0x5d/0xb0
[    0.000000]  ? __SCT__lsm_static_call_bpf_token_capable_7+0x8/0x8
[    0.000000]  ? early_idt_handler_common+0x2f/0x3a
[    0.000000]  ? __pfx_lockdown_is_locked_down+0x10/0x10
[    0.000000]  ? __SCT__lsm_static_call_bpf_token_capable_7+0x8/0x8
[    0.000000]  ? __static_call_update+0x18c/0x1f0
[    0.000000]  ? __static_call_update+0x7e/0x1f0
[    0.000000]  ? sort_r+0x112/0x390
[    0.000000]  ? __pfx_lockdown_is_locked_down+0x10/0x10
[    0.000000]  ? security_add_hooks+0xb8/0x120
[    0.000000]  ? lockdown_lsm_init+0x21/0x30
[    0.000000]  ? initialize_lsm+0x34/0x60
[    0.000000]  ? early_security_init+0x3d/0x50
[    0.000000]  ? start_kernel+0x6b/0xa00
[    0.000000]  ? x86_64_start_reservations+0x24/0x30
[    0.000000]  ? x86_64_start_kernel+0xed/0xf0
[    0.000000]  ? common_startup_64+0x13e/0x141
[    0.000000]  </TASK>
[    0.000000] ---[ end trace 0000000000000000 ]---

Seems like the same problem.

> > > diff --git a/init/main.c b/init/main.c
> > > index 206acdde51f5..5bd45af7a49e 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -922,6 +922,8 @@ void start_kernel(void)
> > >         boot_cpu_init();
> > >         page_address_init();
> > >         pr_notice("%s", linux_banner);
> > > +       /* LSM and command line parameters use static keys */
> > > +       jump_label_init();
> > >         early_security_init();
> > >         setup_arch(&command_line);
> > >         setup_boot_config();
> > > @@ -933,8 +935,6 @@ void start_kernel(void)
> > >         boot_cpu_hotplug_init();
> > >
> > >         pr_notice("Kernel command line: %s\n", saved_command_line);
> > > -       /* parameters may set static keys */
> > > -       jump_label_init();
> > >         parse_early_param();
> > >         after_dashes = parse_args("Booting kernel",
> > >                                   static_command_line, __start___param,
> > > --
> > > 2.46.0.rc2.264.g509ed76dc8-goog
> > 
> > -- 
> > paul-moore.com

Re: [PATCH] init/main.c: Do jump_label_init before early_security_init

[Borislav Petkov]

On Wed, Jul 31, 2024 at 11:34:29PM +0200, KP Singh wrote:
> LSM indirect calls being are now replaced by static calls, this requires
> a jumpt_table_init before early_security_init where LSM hooks and their
> static calls and keys are initialized.
> 
> Fixes: 2732ad5ecd5b ("lsm: replace indirect LSM hook calls with static calls")
> Signed-off-by: KP Singh <kpsingh@kernel.org>
> ---
>  init/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/init/main.c b/init/main.c
> index 206acdde51f5..5bd45af7a49e 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -922,6 +922,8 @@ void start_kernel(void)
>  	boot_cpu_init();
>  	page_address_init();
>  	pr_notice("%s", linux_banner);
> +	/* LSM and command line parameters use static keys */
> +	jump_label_init();
>  	early_security_init();
>  	setup_arch(&command_line);
>  	setup_boot_config();
> @@ -933,8 +935,6 @@ void start_kernel(void)
>  	boot_cpu_hotplug_init();
>  
>  	pr_notice("Kernel command line: %s\n", saved_command_line);
> -	/* parameters may set static keys */
> -	jump_label_init();
>  	parse_early_param();
>  	after_dashes = parse_args("Booting kernel",
>  				  static_command_line, __start___param,
> -- 

I was gonna be very surprised if you could simply change the boot ordering
like that and it would simply work. The early boot order is a nightmare so
without proper audit of what uses which facilities when, you won't be really
successful, I'd say.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] init/main.c: Do jump_label_init before early_security_init

[Peter Zijlstra]

On Wed, Jul 31, 2024 at 09:15:04PM -0400, Paul Moore wrote:
> On Wed, Jul 31, 2024 at 5:34 PM KP Singh <kpsingh@kernel.org> wrote:
> >
> > LSM indirect calls being are now replaced by static calls, this requires
> > a jumpt_table_init before early_security_init where LSM hooks and their
> > static calls and keys are initialized.
> >
> > Fixes: 2732ad5ecd5b ("lsm: replace indirect LSM hook calls with static calls")
> > Signed-off-by: KP Singh <kpsingh@kernel.org>
> > ---
> >  init/main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Does this look okay, static call folks?

Are we confused between jump_label/static_branch and static_call ?

> > diff --git a/init/main.c b/init/main.c
> > index 206acdde51f5..5bd45af7a49e 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -922,6 +922,8 @@ void start_kernel(void)
> >         boot_cpu_init();
> >         page_address_init();
> >         pr_notice("%s", linux_banner);
> > +       /* LSM and command line parameters use static keys */
> > +       jump_label_init();
> >         early_security_init();
> >         setup_arch(&command_line);
> >         setup_boot_config();
> > @@ -933,8 +935,6 @@ void start_kernel(void)
> >         boot_cpu_hotplug_init();
> >
> >         pr_notice("Kernel command line: %s\n", saved_command_line);
> > -       /* parameters may set static keys */
> > -       jump_label_init();
> >         parse_early_param();
> >         after_dashes = parse_args("Booting kernel",
> >                                   static_command_line, __start___param,
> > --
> > 2.46.0.rc2.264.g509ed76dc8-goog
> 
> -- 
> paul-moore.com

Re: [PATCH] init/main.c: Do jump_label_init before early_security_init

[Peter Zijlstra]

On Thu, Aug 01, 2024 at 10:34:41AM +0200, Peter Zijlstra wrote:
> On Wed, Jul 31, 2024 at 09:15:04PM -0400, Paul Moore wrote:
> > On Wed, Jul 31, 2024 at 5:34 PM KP Singh <kpsingh@kernel.org> wrote:
> > >
> > > LSM indirect calls being are now replaced by static calls, this requires
> > > a jumpt_table_init before early_security_init where LSM hooks and their
> > > static calls and keys are initialized.
> > >
> > > Fixes: 2732ad5ecd5b ("lsm: replace indirect LSM hook calls with static calls")
> > > Signed-off-by: KP Singh <kpsingh@kernel.org>
> > > ---
> > >  init/main.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > Does this look okay, static call folks?
> 
> Are we confused between jump_label/static_branch and static_call ?
> 
> > > diff --git a/init/main.c b/init/main.c
> > > index 206acdde51f5..5bd45af7a49e 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -922,6 +922,8 @@ void start_kernel(void)
> > >         boot_cpu_init();
> > >         page_address_init();
> > >         pr_notice("%s", linux_banner);
> > > +       /* LSM and command line parameters use static keys */
> > > +       jump_label_init();
> > >         early_security_init();
> > >         setup_arch(&command_line);
> > >         setup_boot_config();
> > > @@ -933,8 +935,6 @@ void start_kernel(void)
> > >         boot_cpu_hotplug_init();
> > >
> > >         pr_notice("Kernel command line: %s\n", saved_command_line);
> > > -       /* parameters may set static keys */
> > > -       jump_label_init();
> > >         parse_early_param();
> > >         after_dashes = parse_args("Booting kernel",
> > >                                   static_command_line, __start___param,

Anyway, the scariest thing jump_label_init() does is
arch_jump_label_transform_static(). Which, IIRC, was used to optimize
NOPs on x86, which we've since removed.

Only csky and mips seem to still implement this hook, and they do
flush_icache() -- as one would expect.

If any of that is affected by the placement you propose, is something
you'd have to ask those architecture maintainers I'm afraid.

Aside from that I don't see a problem :-)

Re: [PATCH] init/main.c: Do jump_label_init before early_security_init

[KP Singh]

On Thu, Aug 1, 2024 at 10:34 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Jul 31, 2024 at 09:15:04PM -0400, Paul Moore wrote:
> > On Wed, Jul 31, 2024 at 5:34 PM KP Singh <kpsingh@kernel.org> wrote:
> > >
> > > LSM indirect calls being are now replaced by static calls, this requires
> > > a jumpt_table_init before early_security_init where LSM hooks and their
> > > static calls and keys are initialized.
> > >
> > > Fixes: 2732ad5ecd5b ("lsm: replace indirect LSM hook calls with static calls")
> > > Signed-off-by: KP Singh <kpsingh@kernel.org>
> > > ---
> > >  init/main.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Does this look okay, static call folks?
>
> Are we confused between jump_label/static_branch and static_call ?

Not confused, just rushed, from my side, we need both static_call_init
and jump_label_init() before early_security_init(). It fixes the error
for me but as you folks mentioned, we need to check with the arch
folks if this okay.

>
> > > diff --git a/init/main.c b/init/main.c
> > > index 206acdde51f5..5bd45af7a49e 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -922,6 +922,8 @@ void start_kernel(void)
> > >         boot_cpu_init();
> > >         page_address_init();
> > >         pr_notice("%s", linux_banner);
> > > +       /* LSM and command line parameters use static keys */
> > > +       jump_label_init();
> > >         early_security_init();
> > >         setup_arch(&command_line);
> > >         setup_boot_config();
> > > @@ -933,8 +935,6 @@ void start_kernel(void)
> > >         boot_cpu_hotplug_init();
> > >
> > >         pr_notice("Kernel command line: %s\n", saved_command_line);
> > > -       /* parameters may set static keys */
> > > -       jump_label_init();
> > >         parse_early_param();
> > >         after_dashes = parse_args("Booting kernel",
> > >                                   static_command_line, __start___param,
> > > --
> > > 2.46.0.rc2.264.g509ed76dc8-goog
> >
> > --
> > paul-moore.com

Re: [PATCH] init/main.c: Do jump_label_init before early_security_init

[KP Singh]

On Thu, Aug 1, 2024 at 8:14 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Wed, Jul 31, 2024 at 10:48:06PM -0700, Nathan Chancellor wrote:
> > On Wed, Jul 31, 2024 at 09:15:04PM -0400, Paul Moore wrote:
> > > On Wed, Jul 31, 2024 at 5:34 PM KP Singh <kpsingh@kernel.org> wrote:
> > > >
> > > > LSM indirect calls being are now replaced by static calls, this requires
> > > > a jumpt_table_init before early_security_init where LSM hooks and their
> > > > static calls and keys are initialized.
> > > >
> > > > Fixes: 2732ad5ecd5b ("lsm: replace indirect LSM hook calls with static calls")
> > > > Signed-off-by: KP Singh <kpsingh@kernel.org>
> > > > ---
> > > >  init/main.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > Does this look okay, static call folks?
> >
> > For the record, I tested this patch since I noticed the warnings like
> > Boris did and it appears to break booting for me with certain ARCH=arm
> > configurations in QEMU.
> >
> >   $ cat arch/arm/configs/repro.config
> >   CONFIG_JUMP_LABEL=y
> >   CONFIG_SECURITY=y
> >   CONFIG_SECURITY_LOCKDOWN_LSM=y
> >   CONFIG_SECURITY_LOCKDOWN_LSM_EARLY=y
> >
> >   $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- mrproper defconfig repro.config zImage
> >
> >   $ qemu-system-arm \
> >       -display none \
> >       -nodefaults \
> >       -no-reboot \
> >       -machine virt \
> >       -append 'console=ttyAMA0 earlycon' \
> >       -kernel arch/arm/boot/zImage \
> >       -initrd rootfs.cpio \
> >       -m 512m \
> >       -serial mon:stdio
> >   <hangs with no output>
> >
> > Without this patch, that same configuration works fine (with the warning
> > from before):
> >
> >   [    0.000000] Booting Linux on physical CPU 0x0
> >   [    0.000000] Linux version 6.11.0-rc1-next-20240730 (nathan@m3-large-x86) (arm-linux-gnueabi-gcc (GCC) 14.1.0, GNU ld (GNU Binutils) 2.42) #1 SMP Thu Aug  1 05:44:11 UTC 2024
> >   [    0.000000] ------------[ cut here ]------------
> >   [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:199 static_key_enable_cpuslocked+0xb8/0xf4
> >   [    0.000000] static_key_enable_cpuslocked(): static key '0xc1fb4930' used before call to jump_label_init()
> >   [    0.000000] Modules linked in:
> >   [    0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.11.0-rc1-next-20240730 #1
> >   [    0.000000] Call trace:
> >   [    0.000000]  unwind_backtrace from show_stack+0x10/0x14
> >   [    0.000000]  show_stack from dump_stack_lvl+0x54/0x68
> >   [    0.000000]  dump_stack_lvl from __warn+0x80/0x114
> >   [    0.000000]  __warn from warn_slowpath_fmt+0x124/0x18c
> >   [    0.000000]  warn_slowpath_fmt from static_key_enable_cpuslocked+0xb8/0xf4
> >   [    0.000000]  static_key_enable_cpuslocked from static_key_enable+0x14/0x1c
> >   [    0.000000]  static_key_enable from security_add_hooks+0xc4/0xfc
> >   [    0.000000]  security_add_hooks from lockdown_lsm_init+0x18/0x24
> >   [    0.000000]  lockdown_lsm_init from initialize_lsm+0x44/0x7c
> >   [    0.000000]  initialize_lsm from early_security_init+0x44/0x50
> >   [    0.000000]  early_security_init from start_kernel+0x64/0x6bc
> >   [    0.000000]  start_kernel from 0x0
> >   [    0.000000] ---[ end trace 0000000000000000 ]---
> >
> > I haven't tried to fire up GDB to figure out why it is exploding early
> > since it is late for me but I figured I would get the report out first.
> > The rootfs is available from [1] (arm-rootfs.cpio.zst, decompress it
> > with zstd first); it just shuts down the machine on boot.
> >

Thank you so much Nathan! I finally had the time to debug and it seems
like we need setup_arch before early_security_init. I will update my
patch and send it for review.

> > Cheers,
> > Nathan
> >
> > [1]: https://github.com/ClangBuiltLinux/boot-utils/releases/latest
>
> Also, looking at my build logs, this patch does not appear to resolve
> the static call warning I see with certain x86_64 distribution
> configurations such as Fedora's (not sure if it was or not):
>
> https://src.fedoraproject.org/rpms/kernel/raw/rawhide/f/kernel-x86_64-fedora.config
>
> [    0.000000] Linux version 6.11.0-rc1-next-20240730-dirty (nathan@m3-large-x86) (x86_64-linux-gcc (GCC) 14.1.0, GNU ld (GNU Binutils) 2.42) #1 SMP PREEMPT_DYNAMIC Thu Aug  1 06:09:54 UTC 2024
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/static_call_inline.c:153 __static_call_update+0x18c/0x1f0
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.11.0-rc1-next-20240730-dirty #1
> [    0.000000] RIP: 0010:__static_call_update+0x18c/0x1f0
> [    0.000000] Code: 80 3d b6 7b 49 02 00 0f 85 7b ff ff ff 4c 89 f6 48 c7 c7 90 3b bc 8b c6 05 9f 7b 49 02 01 e8 2b 5c da ff 0f 0b e9 5e ff ff ff <0f> 0b 48 c7 c7 40 f2 5f 8c e8 36 72 e4 00 48 8b 44 24 28 65 48 2b
> [    0.000000] RSP: 0000:ffffffff8c403e28 EFLAGS: 00010046 ORIG_RAX: 0000000000000000
> [    0.000000] RAX: 0000000000000000 RBX: ffffffff8b19cd60 RCX: 000000005e199be9
> [    0.000000] RDX: 0000000000000000 RSI: ffffffff8d302a70 RDI: ffffffff8c472500
> [    0.000000] RBP: ffffffff8c6a01a0 R08: 00000000ff5e199b R09: fffffffffffbf82b
> [    0.000000] R10: 0000000000000000 R11: 0000000000013f90 R12: ffffffff8b4d0cb0
> [    0.000000] R13: 0000000000000001 R14: ffffffff8a77e700 R15: 00000000000147d0
> [    0.000000] FS:  0000000000000000(0000) GS:ffffffff8ce3e000(0000) knlGS:0000000000000000
> [    0.000000] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [    0.000000] CR2: ffff8880000147d0 CR3: 000000000af46000 CR4: 00000000000000b0
> [    0.000000] Call Trace:
> [    0.000000]  <TASK>
> [    0.000000]  ? __static_call_update+0x18c/0x1f0
> [    0.000000]  ? __warn.cold+0x93/0xed
> [    0.000000]  ? __static_call_update+0x18c/0x1f0
> [    0.000000]  ? report_bug+0xff/0x140
> [    0.000000]  ? __pfx_lockdown_is_locked_down+0x10/0x10
> [    0.000000]  ? early_fixup_exception+0x5d/0xb0
> [    0.000000]  ? __SCT__lsm_static_call_bpf_token_capable_7+0x8/0x8
> [    0.000000]  ? early_idt_handler_common+0x2f/0x3a
> [    0.000000]  ? __pfx_lockdown_is_locked_down+0x10/0x10
> [    0.000000]  ? __SCT__lsm_static_call_bpf_token_capable_7+0x8/0x8
> [    0.000000]  ? __static_call_update+0x18c/0x1f0
> [    0.000000]  ? __static_call_update+0x7e/0x1f0
> [    0.000000]  ? sort_r+0x112/0x390
> [    0.000000]  ? __pfx_lockdown_is_locked_down+0x10/0x10
> [    0.000000]  ? security_add_hooks+0xb8/0x120
> [    0.000000]  ? lockdown_lsm_init+0x21/0x30
> [    0.000000]  ? initialize_lsm+0x34/0x60
> [    0.000000]  ? early_security_init+0x3d/0x50
> [    0.000000]  ? start_kernel+0x6b/0xa00
> [    0.000000]  ? x86_64_start_reservations+0x24/0x30
> [    0.000000]  ? x86_64_start_kernel+0xed/0xf0
> [    0.000000]  ? common_startup_64+0x13e/0x141
> [    0.000000]  </TASK>
> [    0.000000] ---[ end trace 0000000000000000 ]---
>
> Seems like the same problem.
>
> > > > diff --git a/init/main.c b/init/main.c
> > > > index 206acdde51f5..5bd45af7a49e 100644
> > > > --- a/init/main.c
> > > > +++ b/init/main.c
> > > > @@ -922,6 +922,8 @@ void start_kernel(void)
> > > >         boot_cpu_init();
> > > >         page_address_init();
> > > >         pr_notice("%s", linux_banner);
> > > > +       /* LSM and command line parameters use static keys */
> > > > +       jump_label_init();
> > > >         early_security_init();
> > > >         setup_arch(&command_line);
> > > >         setup_boot_config();
> > > > @@ -933,8 +935,6 @@ void start_kernel(void)
> > > >         boot_cpu_hotplug_init();
> > > >
> > > >         pr_notice("Kernel command line: %s\n", saved_command_line);
> > > > -       /* parameters may set static keys */
> > > > -       jump_label_init();
> > > >         parse_early_param();
> > > >         after_dashes = parse_args("Booting kernel",
> > > >                                   static_command_line, __start___param,
> > > > --
> > > > 2.46.0.rc2.264.g509ed76dc8-goog
> > >
> > > --
> > > paul-moore.com