From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 References: <20190215185151.GG7897@sirena.org.uk> <20190226155948.299aa894a5576e61dda3e5aa@linux-foundation.org> <20190228151438.fc44921e66f2f5d393c8d7b4@linux-foundation.org> <026b5082-32f2-e813-5396-e4a148c813ea@collabora.com> <20190301124100.62a02e2f622ff6b5f178a7c3@linux-foundation.org> <3fafb552-ae75-6f63-453c-0d0e57d818f3@collabora.com> <36faea07-139c-b97d-3585-f7d6d362abc3@collabora.com> <20190306140529.GG3549@rapoport-lnx> <21d138a5-13e4-9e83-d7fe-e0639a8d180a@collabora.com> In-Reply-To: From: Kees Cook Date: Tue, 16 Apr 2019 22:30:53 -0500 Message-ID: Subject: Re: next/master boot bisection: next-20190215 on beaglebone-black Content-Type: text/plain; charset="UTF-8" List-ID: To: Dan Williams Cc: Guenter Roeck , kernelci@groups.io, Guillaume Tucker , Mike Rapoport , Andrew Morton , Michal Hocko , Mark Brown , Tomeu Vizoso , Matt Hart , Stephen Rothwell , Kevin Hilman , Enric Balletbo i Serra , Nicholas Piggin , Dominik Brodowski , Masahiro Yamada , Adrian Reber , Linux Kernel Mailing List , Johannes Weiner , Linux MM , Mathieu Desnoyers , Richard Guy Briggs , "Peter Zijlstra (Intel)" , info@kernelci.org On Tue, Apr 16, 2019 at 4:04 PM Guenter Roeck wrote: > > On Tue, Apr 16, 2019 at 1:37 PM Dan Williams wrote: > > Ah, no, the problem is that jump_label_init() is called by > > setup_arch() on x86, and smp_prepare_boot_cpu() on powerpc, but not > > until after parse_args() on ARM. > > > Anywhere but arm64, x86, and ppc, really. > > $ git grep jump_label_init arch > arch/arm64/kernel/smp.c: jump_label_init(); > arch/powerpc/lib/feature-fixups.c: jump_label_init(); > arch/x86/kernel/setup.c: jump_label_init(); Oooh, nice. Yeah, so, this is already a bug for "hardened_usercopy=0" which sets static branches too. > > Given it appears to be safe to call jump_label_init() early how about > > something like the following? > > > > diff --git a/init/main.c b/init/main.c > > index 598e278b46f7..7d4025d665eb 100644 > > --- a/init/main.c > > +++ b/init/main.c > > @@ -582,6 +582,8 @@ asmlinkage __visible void __init start_kernel(void) > > page_alloc_init(); > > > > pr_notice("Kernel command line: %s\n", boot_command_line); > > + /* parameters may set static keys */ > > + jump_label_init(); > > parse_early_param(); > > after_dashes = parse_args("Booting kernel", > > static_command_line, __start___param, > > @@ -591,8 +593,6 @@ asmlinkage __visible void __init start_kernel(void) > > parse_args("Setting init args", after_dashes, NULL, 0, -1, -1, > > NULL, set_init_arg); > > > > - jump_label_init(); > > - > > That should work, unless there was a reason to have it that late. It > doesn't look like that was the case, but I may be missing something. Yes please. :) Let's fix it like you've suggested. Reviewed-by: Kees Cook -- Kees Cook From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 References: <20190215185151.GG7897@sirena.org.uk> <20190226155948.299aa894a5576e61dda3e5aa@linux-foundation.org> <20190228151438.fc44921e66f2f5d393c8d7b4@linux-foundation.org> <026b5082-32f2-e813-5396-e4a148c813ea@collabora.com> <20190301124100.62a02e2f622ff6b5f178a7c3@linux-foundation.org> <3fafb552-ae75-6f63-453c-0d0e57d818f3@collabora.com> <36faea07-139c-b97d-3585-f7d6d362abc3@collabora.com> <20190306140529.GG3549@rapoport-lnx> <21d138a5-13e4-9e83-d7fe-e0639a8d180a@collabora.com> In-Reply-To: From: Kees Cook Date: Tue, 16 Apr 2019 22:30:53 -0500 Message-ID: Subject: Re: next/master boot bisection: next-20190215 on beaglebone-black Content-Type: text/plain; charset="UTF-8" List-ID: To: Dan Williams Cc: Guenter Roeck , kernelci@groups.io, Guillaume Tucker , Mike Rapoport , Andrew Morton , Michal Hocko , Mark Brown , Tomeu Vizoso , Matt Hart , Stephen Rothwell , Kevin Hilman , Enric Balletbo i Serra , Nicholas Piggin , Dominik Brodowski , Masahiro Yamada , Adrian Reber , Linux Kernel Mailing List , Johannes Weiner , Linux MM , Mathieu Desnoyers , Richard Guy Briggs , "Peter Zijlstra (Intel)" , info@kernelci.org On Tue, Apr 16, 2019 at 4:04 PM Guenter Roeck wrote: > > On Tue, Apr 16, 2019 at 1:37 PM Dan Williams wrote: > > Ah, no, the problem is that jump_label_init() is called by > > setup_arch() on x86, and smp_prepare_boot_cpu() on powerpc, but not > > until after parse_args() on ARM. > > > Anywhere but arm64, x86, and ppc, really. > > $ git grep jump_label_init arch > arch/arm64/kernel/smp.c: jump_label_init(); > arch/powerpc/lib/feature-fixups.c: jump_label_init(); > arch/x86/kernel/setup.c: jump_label_init(); Oooh, nice. Yeah, so, this is already a bug for "hardened_usercopy=0" which sets static branches too. > > Given it appears to be safe to call jump_label_init() early how about > > something like the following? > > > > diff --git a/init/main.c b/init/main.c > > index 598e278b46f7..7d4025d665eb 100644 > > --- a/init/main.c > > +++ b/init/main.c > > @@ -582,6 +582,8 @@ asmlinkage __visible void __init start_kernel(void) > > page_alloc_init(); > > > > pr_notice("Kernel command line: %s\n", boot_command_line); > > + /* parameters may set static keys */ > > + jump_label_init(); > > parse_early_param(); > > after_dashes = parse_args("Booting kernel", > > static_command_line, __start___param, > > @@ -591,8 +593,6 @@ asmlinkage __visible void __init start_kernel(void) > > parse_args("Setting init args", after_dashes, NULL, 0, -1, -1, > > NULL, set_init_arg); > > > > - jump_label_init(); > > - > > That should work, unless there was a reason to have it that late. It > doesn't look like that was the case, but I may be missing something. Yes please. :) Let's fix it like you've suggested. Reviewed-by: Kees Cook -- Kees Cook