Re: [RFC PATCH v2 5/5] panic, crash_core, sys_info: Introduce crash_kexec_in_memory_sys_info

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] crash_core: unconditional printk state reset in __crash_kexec() on failure
- [Medium] panic: permanent printk_panic_seq pin when die() or oops_end() returns
- [Medium] panic: race in printk_clear_panic_start() unpins active Oops or warning
--

commit be914e098a723f2f6a11c63bee2fe8d772bb6b45
Author: Aaron Tomlin <atomlin@atomlin.com>

panic, crash_core, sys_info: Introduce crash_kexec_in_memory_sys_info

This commit introduces the crash_kexec_in_memory_sys_info boot parameter to
collect sys_info() telemetry into the in-memory printk ring buffer before
jumping to the kdump kernel, while suppressing console flushing and pinning
the ring buffer tail at the start of the initial crash message.

> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 2b36aa9fade06..27b73b2da3e5a 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -140,9 +140,15 @@ void __noclone __crash_kexec(struct pt_regs *regs)
>  
>  			crash_setup_regs(&fixed_regs, regs);
>  			crash_save_vmcoreinfo();
> +			if (!crash_kexec_post_notifiers)
> +				panic_in_memory_sys_info();
>  			machine_crash_shutdown(&fixed_regs);
>  			crash_cma_clear_pending_dma();
>  			machine_kexec(kexec_crash_image);
> +			if (!crash_kexec_post_notifiers) {
> +				printk_freeze_tail(false);
> +				printk_suppress_console_flush(false);
> +			}

[Severity: Medium]
In __crash_kexec(), panic_in_memory_sys_info() only calls
printk_suppress_console_flush(true) and printk_freeze_tail(true) when
crash_kexec_in_memory_sys_info is enabled:

kernel/panic.c:panic_in_memory_sys_info()
    if (!crash_kexec_in_memory_sys_info || panic_sys_info_dumped)
        return;
    ...
    printk_suppress_console_flush(true);
    printk_freeze_tail(true);

However, if machine_kexec(kexec_crash_image) fails and returns while
crash_kexec_in_memory_sys_info is disabled (the default), __crash_kexec()
still unconditionally runs printk_freeze_tail(false) and
printk_suppress_console_flush(false).

Calling printk_freeze_tail(false) resets printk_panic_seq to U64_MAX,
discarding the initial Oops start sequence recorded by oops_enter() or
bust_spinlocks(1), and calling printk_suppress_console_flush(false) invokes
printk_trigger_flush() and wake_up_klogd() (queuing irq_work after
machine_crash_shutdown() has already disabled interrupts and APICs) even
though console flushing was never suppressed.

Should the cleanup after machine_kexec() only run when
panic_in_memory_sys_info() actually enabled tail freezing and console flush
suppression?

> diff --git a/kernel/exit.c b/kernel/exit.c
> index 282328d2b4cf8..25facaf6c6f2c 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
[ ... ]
> @@ -1075,6 +1077,14 @@ void __noreturn make_task_dead(int signr)
>  	if (atomic_inc_return(&oops_count) >= limit && limit)
>  		panic("Oopsed too often (kernel.oops_limit is %d)", limit);
>  
> +	while (tsk->in_oops)
> +		bust_spinlocks(0);
> +
> +	if (!READ_ONCE(oops_in_progress) && !panic_in_progress() &&
> +	    ((tsk->flags & PF_EXITING) || !is_global_init(tsk) ||
> +	     atomic_read(&tsk->signal->live) > 1))
> +		printk_clear_panic_start();

[Severity: Medium]
On entry to an Oops, oops_enter() in kernel/panic.c and bust_spinlocks(1) in
lib/bust_spinlocks.c invoke printk_mark_panic_start(), which pins
printk_panic_seq. Neither bust_spinlocks(0) nor oops_exit() clears
printk_panic_seq; for Oops paths, printk_clear_panic_start() is only called
here in make_task_dead().

Across several architectures, when notify_die(DIE_OOPS, ...) returns
NOTIFY_STOP (for example via a registered die notifier such as KGDB/KDB or
kprobes), the trap handler finishes bust_spinlocks(0) and oops_exit() and
returns without calling make_task_dead() or panic():

arch/x86/kernel/dumpstack.c:oops_end()
    bust_spinlocks(0);
    ...
    oops_exit();
    ...
    if (!signr)
        return;

arch/arm64/kernel/traps.c:die()
    bust_spinlocks(0);
    oops_exit();
    ...
    if (ret != NOTIFY_STOP)
        make_task_dead(SIGSEGV);

(And similarly in arch/arm/kernel/traps.c:oops_end(),
arch/riscv/kernel/traps.c:die(), and arch/powerpc/kernel/traps.c:oops_end(),
while arch/loongarch/kernel/traps.c:die() and
arch/hexagon/kernel/traps.c:die() return early on NOTIFY_STOP after
bust_spinlocks(1) without calling bust_spinlocks(0), leaving current->in_oops
and oops_in_progress incremented as well.)

When die() or oops_end() returns without reaching make_task_dead(), won't
printk_panic_seq remain permanently pinned? Once normal printk activity wraps
the ring buffer past that stale sequence number ((u64)panic_seq < first_seq),
a subsequent panic will clamp printk_panic_seq to first_seq at the current
ring buffer tail in printk_mark_panic_start(), leaving zero recyclable space
and causing panic_in_memory_sys_info() to drop all sys_info() output.

> diff --git a/kernel/panic.c b/kernel/panic.c
> index cda5d954b61f3..01175badaba10 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
[ ... ]
> @@ -1067,6 +1096,8 @@ void oops_enter(void)
>  	/* can't trust the integrity of the kernel anymore: */
>  	debug_locks_off();
>  	do_oops_enter_exit();
> +	if (!panic_in_progress())
> +		printk_mark_panic_start();
>  
>  	if (sysctl_oops_all_cpu_backtrace)
>  		trigger_all_cpu_backtrace();
> @@ -1097,10 +1128,16 @@ struct warn_args {
>  void __warn(const char *file, int line, void *caller, unsigned taint,
>  	    struct pt_regs *regs, struct warn_args *args)
>  {
> +	bool clear_panic_start = false;
> +
>  	nbcon_cpu_emergency_enter();
>  
>  	disable_trace_on_warning();
>  
> +	if (!READ_ONCE(oops_in_progress) && !panic_in_progress() &&
> +	    printk_mark_panic_start())
> +		clear_panic_start = true;
[ ... ]
> @@ -1137,6 +1174,9 @@ void __warn(const char *file, int line, void *caller, unsigned taint,
>  	/* Just a warning, don't kill lockdep. */
>  	add_taint(taint, LOCKDEP_STILL_OK);
>  
> +	if (clear_panic_start && !READ_ONCE(oops_in_progress) && !panic_in_progress())
> +		printk_clear_panic_start();

[Severity: Medium]
Can a finishing non-fatal warning in __warn(), warn_slowpath_fmt(), or
__report_bug() (or a task exiting in make_task_dead()) on one CPU race with a
fatal Oops or fatal warning on another CPU and unpin printk_panic_seq?

Specifically, oops_in_progress is 0 and panic_in_progress() is false on CPU B
during three windows before vpanic() sets panic_cpu:

1. In oops_begin() / die(), oops_enter() calls printk_mark_panic_start()
   before bust_spinlocks(1) increments oops_in_progress.

2. In oops_end() / die(), bust_spinlocks(0) decrements oops_in_progress back
   to 0 before oops_exit() and before panic() is called for in_interrupt() or
   panic_on_oops:

   arch/x86/kernel/dumpstack.c:oops_end()
       bust_spinlocks(0);
       ...
       oops_exit();
       __show_regs(&exec_summary_regs, SHOW_REGS_ALL, KERN_DEFAULT);
       if (!signr)
           return;
       if (in_interrupt())
           panic("Fatal exception in interrupt");
       if (panic_on_oops)
           panic("Fatal exception");

3. During a fatal warning (when panic_on_warn, warn_limit, or panic_on_taint
   is set), printk_mark_panic_start() is called while oops_in_progress is 0
   and before check_panic_on_warn() or add_taint() enters panic().

If CPU A finishes a non-fatal warning (having set clear_panic_start = true
just before CPU B started, or while CPU B is in a fatal warning) or runs
make_task_dead() (which does not check whether current pinned
printk_panic_seq) during any of those windows on CPU B, CPU A sees
!READ_ONCE(oops_in_progress) && !panic_in_progress() and calls
printk_clear_panic_start(). When CPU B then enters vpanic(),
printk_mark_panic_start() re-pins printk_panic_seq at the panic banner after
CPU B's Oops or warning output has already been emitted, allowing
panic_in_memory_sys_info() to overwrite the primary crash context.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103024453.922373-1-atomlin@atomlin.com?part=5