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

[Kees Cook]

On Mon, Dec 3, 2018 at 9:42 PM Feng Tang <feng.tang@xxxxxxxxx> 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@xxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxx>
> Cc: stable@xxxxxxxxxx

Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>

-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