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

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: <CAGXu5jJzWSE+LsuwyctS2b0RsYMKcoURkJ_nWXEhBTAcnji_3g@mail.gmail.com>
Date:   Tue, 4 Dec 2018 08:13:24 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Feng Tang <feng.tang@...el.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...e.de>, stable@...nel.org
Subject: Re: [PATCH v3] panic: Avoid the extra noise dmesg

On Mon, Dec 3, 2018 at 9:42 PM Feng Tang <feng.tang@...el.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@...el.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Borislav Petkov <bp@...e.de>
> Cc: stable@...nel.org

Acked-by: Kees Cook <keescook@...omium.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