linux-kernel - [PATCH v3] printk: Relocate wake_klogd check close to the end of console

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]

Message-ID: <20180219160122.upqkpbbffm3rth6p@pathway.suse.cz>
Date:   Mon, 19 Feb 2018 17:01:22 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        linux-kernel@...r.kernel.org, Tejun Heo <tj@...nel.org>
Subject: [PATCH v3] printk: Relocate wake_klogd check close to the end of
 console_unlock()

From: Tejun Heo <tj@...nel.org>

We mark for waking up klogd whenever we see a new message sequence in
the main loop.  However, the actual wakeup is always at the end of the
function and we can easily test for the wakeup condition when we do
the final should-we-repeat check.

Move the wake_klogd condition check out of the main loop.  This avoids
doing the same thing repeatedly and groups similar checks into a
common place.

This fixes a race introduced by the commit dbdda842fe96f8932 ("printk: Add
console owner and waiter logic to load balance console writes").
The current console owner might process the newly added message before
the related printk() start waiting for the console lock. Then the lock
is passed without waking klogd. The new owner sees the already updated
seen_seq and does not know that the wakeup is needed.

Fixes: dbdda842fe96f8932 ("printk: Add console owner and waiter logic to load balance console writes")
Signed-off-by: Tejun Heo <tj@...nel.org>
[pmladek@...e.com: Fix and describe a new race with console waiter]
Signed-off-by: Petr Mladek <pmladek@...e.com>
Cc: Petr Mladek <pmladek@...e.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc: Steven Rostedt <rostedt@...dmis.org>
---
Changes against v2:

+ check wake_klogd only when really leaving

 kernel/printk/printk.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index db4b9b8929eb..aaa28c1409ab 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2335,10 +2335,6 @@ void console_unlock(void)
 
 		printk_safe_enter_irqsave(flags);
 		raw_spin_lock(&logbuf_lock);
-		if (seen_seq != log_next_seq) {
-			wake_klogd = true;
-			seen_seq = log_next_seq;
-		}
 
 		if (console_seq < log_first_seq) {
 			len = sprintf(text, "** %u printk messages dropped **\n",
@@ -2416,14 +2412,22 @@ void console_unlock(void)
 
 	up_console_sem();
 
+	raw_spin_lock(&logbuf_lock);
 	/*
 	 * Someone could have filled up the buffer again, so re-check if there's
 	 * something to flush. In case we cannot trylock the console_sem again,
 	 * there's a new owner and the console_unlock() from them will do the
 	 * flush, no worries.
 	 */
-	raw_spin_lock(&logbuf_lock);
 	retry = console_seq != log_next_seq;
+	/*
+	 * Check whether userland needs notification. Do this only when really
+	 * leaving to avoid race with console_trylock_spinning().
+	 */
+	if (seen_seq != log_next_seq && !retry) {
+		wake_klogd = true;
+		seen_seq = log_next_seq;
+	}
 	raw_spin_unlock(&logbuf_lock);
 	printk_safe_exit_irqrestore(flags);
 
-- 
2.13.6