| 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
| ||
|
Date: Fri, 1 Mar 2019 16:57:52 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Feng Tang <feng.tang@...el.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Petr Mladek <pmladek@...e.com>, linux-kernel@...r.kernel.org,
Thomas Gleixner <tglx@...utronix.de>,
Kees Cook <keescook@...omium.org>,
Borislav Petkov <bp@...e.de>,
Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
Andi Kleen <ak@...ux.intel.com>,
Peter Zijlstra <peterz@...radead.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jslaby@...e.com>, Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH v6] panic: Avoid the extra noise dmesg
On Fri, 1 Mar 2019 16:49:46 +0800
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 overridden 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.
>
> Adding a flag "suppress_printk" which gets set in panic() to avoid those
> noisy messages, without changing current kernel behavior that both panic
> blinking and sysrq magic key can work as is, suggested by Petr Mladek.
>
> To verify this, make sure kernel is not configured to reboot on panic and
> in console
> # echo c > /proc/sysrq-trigger
> to see if console only prints out the panic call stack.
>
Looks good to me.
Acked-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
Andrew, you want to take this patch?
-- Steve
> Suggested-by: Petr Mladek <pmladek@...e.com>
> 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: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
> Cc: Petr Mladek <pmladek@...e.com>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Andi Kleen <ak@...ux.intel.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Jiri Slaby <jslaby@...e.com>
> Cc: Sasha Levin <sashal@...nel.org>
> ---
> Change log:
>
> v6:
> - change the flag name from "panic_suppress_printk" to
> suppress_printk, as suggested by Andrew Morton
>
> v5:
> - use a flag to notify printk not to print unimportant
> messages, while keeping panic blinking and sysrq working,
> as suggested by Petr Mladek
>
> v4:
> - make the local_irq_enable conditional and default off
> to cover possible use of interrupt/scheduling, as
> mentioned by Sergey and Petr
>
> v3:
> - Make the change log clearer as suggested by Andrew Morton
>
> v2:
> - Move the solution from hacking arch/scheduler code back
> to panic.c
>
> drivers/tty/sysrq.c | 6 ++++++
> include/linux/printk.h | 2 ++
> kernel/panic.c | 3 +++
> kernel/printk/printk.c | 10 ++++++++++
> 4 files changed, 21 insertions(+)
>
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 1f03078..ddc2d8a 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -527,8 +527,12 @@ void __handle_sysrq(int key, bool check_mask)
> {
> struct sysrq_key_op *op_p;
> int orig_log_level;
> + int orig_suppress_printk;
> int i;
>
> + orig_suppress_printk = suppress_printk;
> + suppress_printk = 0;
> +
> rcu_sysrq_start();
> rcu_read_lock();
> /*
> @@ -574,6 +578,8 @@ void __handle_sysrq(int key, bool check_mask)
> }
> rcu_read_unlock();
> rcu_sysrq_end();
> +
> + suppress_printk = orig_suppress_printk;
> }
>
> void handle_sysrq(int key)
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index 77740a5..a010cdf 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -83,6 +83,8 @@ static inline void console_verbose(void)
> extern char devkmsg_log_str[];
> struct ctl_table;
>
> +extern int suppress_printk;
> +
> struct va_format {
> const char *fmt;
> va_list *va;
> diff --git a/kernel/panic.c b/kernel/panic.c
> index f121e6b..960fdfa 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -326,6 +326,9 @@ void panic(const char *fmt, ...)
> }
> #endif
> pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
> +
> + /* Do not scroll important messages printed above */
> + suppress_printk = 1;
> local_irq_enable();
> for (i = 0; ; i += PANIC_TIMER_STEP) {
> touch_softlockup_watchdog();
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index d3d1703..8f1c19f 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -85,6 +85,12 @@ static DEFINE_SEMAPHORE(console_sem);
> struct console *console_drivers;
> EXPORT_SYMBOL_GPL(console_drivers);
>
> +/*
> + * System may need to suppress printk message under certain
> + * circumstances, like after kernel panic happens.
> + */
> +int __read_mostly suppress_printk;
> +
> #ifdef CONFIG_LOCKDEP
> static struct lockdep_map console_lock_dep_map = {
> .name = "console_lock"
> @@ -1899,6 +1905,10 @@ asmlinkage int vprintk_emit(int facility, int level,
> unsigned long flags;
> u64 curr_log_seq;
>
> + /* Suppress unimportant messages after panic happens */
> + if (unlikely(suppress_printk))
> + return 0;
> +
> if (level == LOGLEVEL_SCHED) {
> level = LOGLEVEL_DEFAULT;
> in_sched = true;
Powered by blists - more mailing lists