Re: [PATCH 2/4] kexec: add CRASH_ZEROIZE to wipe secrets before kdump

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Deadlock in `crash_zeroize()` due to `ktime_get()` during kdump panic path
- [Low] Missing registration wrapper for conditionally compiled `crash_zeroize_notifier_list`

Pre-existing issues:
- [High] Pre-existing deadlock risk in `crash_save_vmcoreinfo` on 32-bit platforms
--

--- Patch [2]: [PATCH 2/4] kexec: add CRASH_ZEROIZE to wipe secrets before kdump ---
commit 3005bf8ed21843217c4ae3efeb7db1241b960e62
Author: Jan Sebastian Götte <linux@jaseg.de>

kexec: add CRASH_ZEROIZE to wipe secrets before kdump

When kdump is used to capture system memory after a panic(), any secret
keys currently in RAM end up in the dump. Add an opt-in atomic notifier
chain, crash_zeroize_notifier_list, invoked late into __crash_kexec().
Subsystems holding secrets can register a callback to scrub them.

> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h

[ ... ]

> @@ -34,6 +35,10 @@ static inline void arch_kexec_protect_crashkres(void) { }
>  static inline void arch_kexec_unprotect_crashkres(void) { }
>  #endif
>  
> +#ifdef CONFIG_CRASH_ZEROIZE
> +extern struct atomic_notifier_head crash_zeroize_notifier_list;
> +#endif

[Severity: Low]
Is it possible to provide a registration wrapper instead of exporting the
list directly? 

If developers try to register a callback using 
atomic_notifier_chain_register() when CONFIG_CRASH_ZEROIZE is disabled, they
will encounter a build failure unless they wrap the call in #ifdef guards.
Adding a wrapper like register_crash_zeroize_notifier() with a static inline
stub for the !CONFIG case would avoid this.

> +
>  #ifndef arch_crash_handle_hotplug_event
>  static inline void arch_crash_handle_hotplug_event(struct kimage *image, void *arg) { }
>  #endif

[ ... ]

> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c

[ ... ]

> @@ -33,6 +34,22 @@
>  /* Per cpu memory for storing cpu states in case of system crash. */
>  note_buf_t __percpu *crash_notes;
>  
> +#ifdef CONFIG_CRASH_ZEROIZE
> +ATOMIC_NOTIFIER_HEAD(crash_zeroize_notifier_list);
> +EXPORT_SYMBOL_GPL(crash_zeroize_notifier_list);
> +
> +static void crash_zeroize(void)
> +{
> +	ktime_t zeroize_start = ktime_get();

[Severity: High]
Can this ktime_get() call cause a deadlock? 

When panic() triggers kdump, it calls machine_crash_shutdown() to stop all
other CPUs. Since crash_zeroize() executes immediately after this shutdown
step, if any other CPU was executing a timekeeping update and holding
tk_core.seq for writing when it was permanently stopped by an NMI/IPI, the
panicking CPU will spin forever in read_seqcount_begin().

This would entirely prevent the crash dump from being collected. Should this
use a lockless accessor like ktime_get_mono_fast_ns() instead?

> +
> +	pr_info("Wiping sensitive secrets...\n");
> +	atomic_notifier_call_chain(&crash_zeroize_notifier_list, 0, NULL);
> +	pr_info("Done in %lld us\n", ktime_us_delta(ktime_get(), zeroize_start));
> +}
> +#else
> +static inline void crash_zeroize(void) { }
> +#endif /* CONFIG_CRASH_ZEROIZE */
> +

[ ... ]

> @@ -142,6 +159,7 @@ void __noclone __crash_kexec(struct pt_regs *regs)
>  			crash_save_vmcoreinfo();

[Severity: High]
This is a pre-existing issue, but does crash_save_vmcoreinfo() carry a
similar deadlock risk on 32-bit platforms?

Looking at crash_save_vmcoreinfo(), it calls ktime_get_real_seconds(), which
relies on the tk_core.seq sequence lock on 32-bit architectures. 

If the panicking CPU interrupted a timekeeping update, or if another CPU was
stopped while holding the lock, this call will spin forever and block kdump.
Using __ktime_get_real_seconds() would bypass the sequence lock.

>  			machine_crash_shutdown(&fixed_regs);
>  			crash_cma_clear_pending_dma();
> +			crash_zeroize();
>  			machine_kexec(kexec_crash_image);
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731162739.158320-1-linux@jaseg.de?part=2