linux-kernel - Re: [PATCH] powerpc/fadump, x86/sev: Inform about unconditionally enabling crash_kexec_post

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] [day] [month] [year] [list]

Message-ID: <d1e65436-b49b-76b1-01d4-2be2a0eb0f3a@igalia.com>
Date: Fri, 30 Aug 2024 15:11:13 -0300
From: "Guilherme G. Piccoli" <gpiccoli@...lia.com>
To: Stephen Brennan <stephen.s.brennan@...cle.com>,
 kexec@...ts.infradead.org, linuxppc-dev@...ts.ozlabs.org, x86@...nel.org
Cc: bhe@...hat.com, vgoyal@...hat.com, dyoung@...hat.com, mpe@...erman.id.au,
 npiggin@...il.com, christophe.leroy@...roup.eu, naveen@...nel.org,
 hbathini@...ux.ibm.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, hpa@...or.com, ashish.kalra@....com,
 michael.roth@....com, brijesh.singh@....com, thomas.lendacky@....com,
 linux-kernel@...r.kernel.org, linux-debuggers@...r.kernel.org,
 kernel@...ccoli.net, kernel-dev@...lia.com
Subject: Re: [PATCH] powerpc/fadump, x86/sev: Inform about unconditionally
 enabling crash_kexec_post_notifiers

On 30/08/2024 14:54, Stephen Brennan wrote:
> [...]
> Could we maybe go further than this, and delete the public declarations
> of crash_kexec_post_notifiers in "include/linux"? (I see two). We could
> replace the users that set it to true with a function that logs the
> change so that it's impossible for new code to set it directly without
> notifying the user. Something like this? Compile tested only for x86.
> 
> commit da8691a25d7b0c2f914720bc054dd1d9dbe4b373
> Author: Stephen Brennan <stephen.s.brennan@...cle.com>
> Date:   Fri Aug 30 10:49:24 2024 -0700
> 
>     panic: make crash_kexec_post_notifiers private
>     
>     This requires that any in-kernel user setting it directly must log the
>     reason so that users are aware their panic behavior may be different
>     from their configuration.
>     
>     Signed-off-by: Stephen Brennan <stephen.s.brennan@...cle.com>
> 

Thanks Stephen! I'm totally into that, your approach is very good.
Cheers,


Guilherme


> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index a612e7513a4f8..9966f29409599 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -1818,7 +1818,7 @@ int __init setup_fadump(void)
>  	 * lets panic() function take crash friendly path before panic
>  	 * notifiers are invoked.
>  	 */
> -	crash_kexec_post_notifiers = true;
> +	enable_crash_kexec_post_notifiers("PPC/fadump");
>  
>  	return 1;
>  }
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 0ce17766c0e52..6e9f5f8d13cc5 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -256,7 +256,7 @@ static int __init snp_rmptable_init(void)
>  	 * Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
>  	 * notifier is invoked to do SNP IOMMU shutdown before kdump.
>  	 */
> -	crash_kexec_post_notifiers = true;
> +	enable_crash_kexec_post_notifiers("AMD/SEV");
>  
>  	return 0;
>  
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 9c452bfbd5719..fa3bbb66235de 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -303,8 +303,7 @@ int __init hv_common_init(void)
>  	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
>  		u64 hyperv_crash_ctl;
>  
> -		crash_kexec_post_notifiers = true;
> -		pr_info("Hyper-V: enabling crash_kexec_post_notifiers\n");
> +		enable_crash_kexec_post_notifiers("Hyper-V");
>  
>  		/*
>  		 * Panic message recording (sysctl_record_panic_msg)
> diff --git a/include/linux/panic.h b/include/linux/panic.h
> index 54d90b6c5f47b..697184664c6f4 100644
> --- a/include/linux/panic.h
> +++ b/include/linux/panic.h
> @@ -31,8 +31,6 @@ extern int sysctl_panic_on_rcu_stall;
>  extern int sysctl_max_rcu_stall_to_panic;
>  extern int sysctl_panic_on_stackoverflow;
>  
> -extern bool crash_kexec_post_notifiers;
> -
>  extern void __stack_chk_fail(void);
>  void abort(void);
>  
> diff --git a/include/linux/panic_notifier.h b/include/linux/panic_notifier.h
> index 41e32483d7a7b..97c31cf5c2fdb 100644
> --- a/include/linux/panic_notifier.h
> +++ b/include/linux/panic_notifier.h
> @@ -7,6 +7,6 @@
>  
>  extern struct atomic_notifier_head panic_notifier_list;
>  
> -extern bool crash_kexec_post_notifiers;
> +void enable_crash_kexec_post_notifiers(const char *reason);
>  
>  #endif	/* _LINUX_PANIC_NOTIFIERS_H */
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 63cf89393c6eb..634c6b99717c5 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -33,6 +33,9 @@
>  /* Per cpu memory for storing cpu states in case of system crash. */
>  note_buf_t __percpu *crash_notes;
>  
> +/* Defined in kernel/panic.c and needed here, but not intended to be public. */
> +extern bool crash_kexec_post_notifiers;
> +
>  #ifdef CONFIG_CRASH_DUMP
>  
>  int kimage_crash_copy_vmcoreinfo(struct kimage *image)
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 2a0449144f82e..f4ae3abbea7ed 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -137,6 +137,12 @@ static long no_blink(int state)
>  	return 0;
>  }
>  
> +void enable_crash_kexec_post_notifiers(const char *reason)
> +{
> +	crash_kexec_post_notifiers = true;
> +	pr_info("%s: enabling crash_kexec_post_notifiers\n", reason);
> +}
> +
>  /* Returns how long it waited in ms */
>  long (*panic_blink)(int state);
>  EXPORT_SYMBOL(panic_blink);
>