linux-kernel - Re: [PATCH 01/11] init: Mark [arch_call_]rest_init() _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZDRrW9+FrGxgw6Ok@google.com>
Date:   Mon, 10 Apr 2023 13:02:35 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Josh Poimboeuf <jpoimboe@...nel.org>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Miroslav Benes <mbenes@...e.cz>, linux-btrfs@...r.kernel.org,
        Mark Rutland <mark.rutland@....com>,
        linux-scsi@...r.kernel.org, linux-hyperv@...r.kernel.org,
        Arnd Bergmann <arnd@...db.de>,
        "Guilherme G . Piccoli" <gpiccoli@...lia.com>,
        Michael Kelley <mikelley@...rosoft.com>, nathan@...nel.org,
        llvm@...ts.linux.dev
Subject: Re: [PATCH 01/11] init: Mark [arch_call_]rest_init() __noreturn

On Fri, Apr 07, 2023 at 05:09:54PM -0700, Josh Poimboeuf wrote:
> Fixes the following warning:
> 
>   init/main.o: warning: objtool: start_kernel+0x4ad: unreachable instruction
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@...nel.org>

Ha! I was just looking into this, by coincidence.
https://github.com/ClangBuiltLinux/linux/issues/1815#issuecomment-1489113303

Generally, the compiler can infer when a function does not return, but
it's not safe to do so when the definition is weak, since the strong
definition may not be, so the caller must not make optimizations based
on such an assumption.

https://reviews.llvm.org/D147177 (abandoned, but has more info)

Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com>

(I agree with the first two patches, since those are ones I was just
looking at.  Unfortunately, some security embargo just lifted on
https://bugs.chromium.org/p/llvm/issues/detail?id=30 which a clang-16
change is causing the stack canaries to be set+checked again in
start_kernel. I'm about to meet with LLVM folks at Intel in 30 minutes
to discuss walking back some of the changes since I strongly disagree
with them.  But merging this series will cause boot failures with
clang-16, and I would appreciate some time to sort this out before this
series is merged. It's my top priority.)

> ---
>  arch/s390/kernel/setup.c     | 2 +-
>  include/linux/start_kernel.h | 4 ++--
>  init/main.c                  | 4 ++--
>  tools/objtool/check.c        | 2 ++
>  4 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
> index 8ec5cdf9dadc..4259b6c50516 100644
> --- a/arch/s390/kernel/setup.c
> +++ b/arch/s390/kernel/setup.c
> @@ -396,7 +396,7 @@ int __init arch_early_irq_init(void)
>  	return 0;
>  }
>  
> -void __init arch_call_rest_init(void)
> +void __init __noreturn arch_call_rest_init(void)
>  {
>  	unsigned long stack;
>  
> diff --git a/include/linux/start_kernel.h b/include/linux/start_kernel.h
> index 8b369a41c03c..864921e54c92 100644
> --- a/include/linux/start_kernel.h
> +++ b/include/linux/start_kernel.h
> @@ -9,7 +9,7 @@
>     up something else. */
>  
>  extern asmlinkage void __init start_kernel(void);
> -extern void __init arch_call_rest_init(void);
> -extern void __ref rest_init(void);
> +extern void __init __noreturn arch_call_rest_init(void);
> +extern void __ref __noreturn rest_init(void);
>  
>  #endif /* _LINUX_START_KERNEL_H */
> diff --git a/init/main.c b/init/main.c
> index 4425d1783d5c..161ed956d738 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -683,7 +683,7 @@ static void __init setup_command_line(char *command_line)
>  
>  static __initdata DECLARE_COMPLETION(kthreadd_done);
>  
> -noinline void __ref rest_init(void)
> +noinline void __ref __noreturn rest_init(void)
>  {
>  	struct task_struct *tsk;
>  	int pid;
> @@ -889,7 +889,7 @@ static int __init early_randomize_kstack_offset(char *buf)
>  early_param("randomize_kstack_offset", early_randomize_kstack_offset);
>  #endif
>  
> -void __init __weak arch_call_rest_init(void)
> +void __init __weak __noreturn arch_call_rest_init(void)
>  {
>  	rest_init();
>  }
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index cae6ac6ff246..e7442e7ad676 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -202,6 +202,7 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
>  		"__reiserfs_panic",
>  		"__stack_chk_fail",
>  		"__ubsan_handle_builtin_unreachable",
> +		"arch_call_rest_init",
>  		"arch_cpu_idle_dead",
>  		"cpu_bringup_and_idle",
>  		"cpu_startup_entry",
> @@ -217,6 +218,7 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
>  		"machine_real_restart",
>  		"make_task_dead",
>  		"panic",
> +		"rest_init",
>  		"rewind_stack_and_make_dead",
>  		"sev_es_terminate",
>  		"snp_abort",
> -- 
> 2.39.2
> 
>