[PATCH v3] panic: Avoid the extra noise dmesg

[PATCH v3] panic: Avoid the extra noise dmesg

[Feng Tang]

When kernel panic happens, it will first print the panic call stack,
then the ending msg like:

[   35.743249] ---[ end Kernel panic - not syncing: Fatal exception
[   35.749975] ------------[ cut here ]------------

The above message are very useful for debugging.

But if system is configured to not reboot on panic, say the "panic_timeout"
parameter equals 0, it will likely print out many noisy message like
WARN() call stack for each and every CPU except the panic one, messages
like below:

	WARNING: CPU: 1 PID: 280 at kernel/sched/core.c:1198 set_task_cpu+0x183/0x190
	Call Trace:
	<IRQ>
	try_to_wake_up
	default_wake_function
	autoremove_wake_function
	__wake_up_common
	__wake_up_common_lock
	__wake_up
	wake_up_klogd_work_func
	irq_work_run_list
	irq_work_tick
	update_process_times
	tick_sched_timer
	__hrtimer_run_queues
	hrtimer_interrupt
	smp_apic_timer_interrupt
	apic_timer_interrupt

For people working in console mode, the screen will first show the panic
call stack, but immediately overridded by these noisy extra messages, which
makes debugging much more difficult, as the original context gets lost on
screen.

Also these noisy messages will confuse some users, as I have seen many bug
reporters posted the noisy message into bugzilla, instead of the real panic
call stack and context.

Removing the "local_irq_enable" will avoid the noisy message.

The justification for the removing is: when code runs to this point, it
means user has chosed to not reboot, or do any special handling by using
the panic notifier method, no much point in re-enabling the interrupt.

Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kees Cook <keescook@chromium.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: stable@kernel.org
---
 kernel/panic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index f6d549a..a616e55 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -295,7 +295,6 @@ void panic(const char *fmt, ...)
 	}
 #endif
 	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
-	local_irq_enable();
 	for (i = 0; ; i += PANIC_TIMER_STEP) {
 		touch_softlockup_watchdog();
 		if (i >= i_next) {
-- 
2.7.4

Re: [PATCH v3] panic: Avoid the extra noise dmesg

[Kees Cook]

On Mon, Dec 3, 2018 at 9:42 PM Feng Tang <feng.tang@intel.com> wrote:
>
> When kernel panic happens, it will first print the panic call stack,
> then the ending msg like:
>
> [   35.743249] ---[ end Kernel panic - not syncing: Fatal exception
> [   35.749975] ------------[ cut here ]------------
>
> The above message are very useful for debugging.
>
> But if system is configured to not reboot on panic, say the "panic_timeout"
> parameter equals 0, it will likely print out many noisy message like
> WARN() call stack for each and every CPU except the panic one, messages
> like below:
>
>         WARNING: CPU: 1 PID: 280 at kernel/sched/core.c:1198 set_task_cpu+0x183/0x190
>         Call Trace:
>         <IRQ>
>         try_to_wake_up
>         default_wake_function
>         autoremove_wake_function
>         __wake_up_common
>         __wake_up_common_lock
>         __wake_up
>         wake_up_klogd_work_func
>         irq_work_run_list
>         irq_work_tick
>         update_process_times
>         tick_sched_timer
>         __hrtimer_run_queues
>         hrtimer_interrupt
>         smp_apic_timer_interrupt
>         apic_timer_interrupt
>
> For people working in console mode, the screen will first show the panic
> call stack, but immediately overridded by these noisy extra messages, which
> makes debugging much more difficult, as the original context gets lost on
> screen.
>
> Also these noisy messages will confuse some users, as I have seen many bug
> reporters posted the noisy message into bugzilla, instead of the real panic
> call stack and context.
>
> Removing the "local_irq_enable" will avoid the noisy message.
>
> The justification for the removing is: when code runs to this point, it
> means user has chosed to not reboot, or do any special handling by using
> the panic notifier method, no much point in re-enabling the interrupt.
>
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: stable@kernel.org

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  kernel/panic.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index f6d549a..a616e55 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -295,7 +295,6 @@ void panic(const char *fmt, ...)
>         }
>  #endif
>         pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
> -       local_irq_enable();
>         for (i = 0; ; i += PANIC_TIMER_STEP) {
>                 touch_softlockup_watchdog();
>                 if (i >= i_next) {
> --
> 2.7.4
>


-- 
Kees Cook