linux-kernel - Re: Regression in v4.19.106 breaking waking up of readers of /proc/kmsg and /dev/kmsg

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] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 29 Feb 2020 13:08:06 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     "Theodore Y. Ts'o" <tytso@....edu>, Petr Mladek <pmladek@...e.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Lech Perczak <l.perczak@...lintechnologies.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Krzysztof Drobiński 
        <k.drobinski@...lintechnologies.com>,
        Pawel Lenkow <p.lenkow@...lintechnologies.com>,
        John Ogness <john.ogness@...utronix.de>,
        Tejun Heo <tj@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: Regression in v4.19.106 breaking waking up of readers of
 /proc/kmsg and /dev/kmsg

On (20/02/29 12:32), Sergey Senozhatsky wrote:
> > I'm wondering now if we should revert this commit before 5.6 comes out
> > (it landed in 5.6-rc1).  "Is much less likely to happen given the
> > other random initialization patches" is not the same as "guaranteed
> > not to happen".
> > 
> > What do folks think?
> 
> Well, my 5 cents, there is nothing that prevents "too-early"
> printk_deferred() calls in the future. From that POV I'd probably
> prefer to "forbid" printk_deffered() to touch per-CPU deferred
> machinery until it's not "too early" anymore. Similar to what we
> do in printk_safe::queue_flush_work().

Informal patch

=====
Subject: [PATCH] printk: defer printk_deferred() until irq_work is ready

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
---
 kernel/printk/internal.h    |  3 +++
 kernel/printk/printk.c      |  6 ++++++
 kernel/printk/printk_safe.c | 11 ++++++++---
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index c8e6ab689d42..2f051fb83c00 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -23,6 +23,8 @@ __printf(1, 0) int vprintk_func(const char *fmt, va_list args);
 void __printk_safe_enter(void);
 void __printk_safe_exit(void);
 
+bool printk_irq_work_ready(void);
+
 #define printk_safe_enter_irqsave(flags)	\
 	do {					\
 		local_irq_save(flags);		\
@@ -64,4 +66,5 @@ __printf(1, 0) int vprintk_func(const char *fmt, va_list args) { return 0; }
 #define printk_safe_enter_irq() local_irq_disable()
 #define printk_safe_exit_irq() local_irq_enable()
 
+bool printk_irq_work_ready(void) { return false; }
 #endif /* CONFIG_PRINTK */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index ad4606234545..bb545c86124e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3009,6 +3009,9 @@ static DEFINE_PER_CPU(struct irq_work, wake_up_klogd_work) = {
 
 void wake_up_klogd(void)
 {
+	if (!printk_irq_work_ready())
+		return;
+
 	preempt_disable();
 	if (waitqueue_active(&log_wait)) {
 		this_cpu_or(printk_pending, PRINTK_PENDING_WAKEUP);
@@ -3019,6 +3022,9 @@ void wake_up_klogd(void)
 
 void defer_console_output(void)
 {
+	if (!printk_irq_work_ready())
+		return;
+
 	preempt_disable();
 	__this_cpu_or(printk_pending, PRINTK_PENDING_OUTPUT);
 	irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c
index b4045e782743..5d2c282984d0 100644
--- a/kernel/printk/printk_safe.c
+++ b/kernel/printk/printk_safe.c
@@ -27,7 +27,7 @@
  * There are situations when we want to make sure that all buffers
  * were handled or when IRQs are blocked.
  */
-static int printk_safe_irq_ready __read_mostly;
+static bool __printk_irq_work_ready __read_mostly;
 
 #define SAFE_LOG_BUF_LEN ((1 << CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT) -	\
 				sizeof(atomic_t) -			\
@@ -48,10 +48,15 @@ static DEFINE_PER_CPU(int, printk_context);
 static DEFINE_PER_CPU(struct printk_safe_seq_buf, nmi_print_seq);
 #endif
 
+bool printk_irq_work_ready(void)
+{
+	return __printk_irq_work_ready;
+}
+
 /* Get flushed in a more safe context. */
 static void queue_flush_work(struct printk_safe_seq_buf *s)
 {
-	if (printk_safe_irq_ready)
+	if (printk_irq_work_ready())
 		irq_work_queue(&s->work);
 }
 
@@ -408,7 +413,7 @@ void __init printk_safe_init(void)
 	 * variable is set.
 	 */
 	barrier();
-	printk_safe_irq_ready = 1;
+	__printk_irq_work_ready = true;
 
 	/* Flush pending messages that did not have scheduled IRQ works. */
 	printk_safe_flush();
-- 
2.25.0.265.gbab2e86ba0-goog