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:	Thu, 14 Jan 2016 14:59:18 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Tejun Heo <tj@...nel.org>, Jan Kara <jack@...e.com>,
	Petr Mladek <pmladek@...e.com>,
	Kyle McMartin <kyle@...nel.org>,
	Dave Jones <davej@...emonkey.org.uk>,
	Calvin Owens <calvinowens@...com>,
	linux-kernel@...r.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: [PATCH 1/2] printk: move can_use_console out of console_trylock_for_printk

vprintk_emit() disables preemption around console_trylock_for_printk()
and console_unlock() calls for a strong reason -- can_use_console()
check. The thing is that vprintl_emit() can be called on a CPU that
is not fully brought up yet (!cpu_online()), which potentially can
cause problems if console driver accesses per-cpu data. A console
driver can explicitly state that it's safe to call it from !online
cpu by setting CON_ANYTIME bit in console ->flags. That's why for
!cpu_online() can_use_console() iterates all the console to find out
if there is a CON_ANYTIME console, otherwise console_unlock() must be
avoided.

call_console_drivers(), called from console_cont_flush() and
console_unlock(), does the same test during for_each_console() loop.
However, we can have the following corner case. Assume that we have 2
cpus -- CPU0 is online, CPU1 is !online; and no CON_ANYTIME consoles
available.

CPU0 online                        CPU1 !online
                                 console_trylock()
                                 ...
                                 console_unlock()
                                   console_cont_flush
                                     spin_lock logbuf_lock
                                     if (!cont.len) {
                                        spin_unlock logbuf_lock
                                        return
                                     }
                                   for (;;) {
vprintk_emit
  spin_lock logbuf_lock
  log_store
  spin_unlock logbuf_lock
                                     spin_lock logbuf_lock
  !console_trylock_for_printk        msg_print_text
 return                              console_idx = log_next()
                                     console_seq++
                                     console_prev = msg->flags
                                     spin_unlock logbuf_lock

                                     call_console_drivers()
                                       for_each_console(con) {
                                         if (!cpu_online() &&
                                             !(con->flags & CON_ANYTIME))
                                                 continue;
                                         }
                                   /*
                                    * no message printed, we lost it
                                    */
vprintk_emit
  spin_lock logbuf_lock
  log_store
  spin_unlock logbuf_lock
  !console_trylock_for_printk
 return
                                   /*
                                    * go to the beginning of the loop,
                                    * find out there are new messages,
                                    * lose it
                                    */
                                   }

This patch moves can_use_console() check out of
console_trylock_for_printk(). Instead it calls it in two
non-preemptible sections:
1) console_cont_flush()
   can_use_console() call happens with spin_lock logbuf_lock
   taken and local IRQs disabled, local IRQs are not enabled
   until call_console_drivers() returns.

2) console_unlock()
   can_use_console() call happens with spin_lock logbuf_lock
   taken and local IRQs disabled, local IRQs are not enabled
   until call_console_drivers() returns.

In both cases, we need to know that call_console_drivers() call
was aborted because of !can_use_console() and we must exit
console_unlock() function.

This also lets to drop '!cpu_online() && !(con->flags & CON_ANYTIME)'
test from call_console_drivers().

At the same time this patch serves as a preparation to enable
cond_resched for 'in-direct' console_unlock() callers -- the ones
that come from vprintk_emit(). The 'direct' ones -- the ones that
do console_lock()/console_unlock() -- already can cond_resched.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
---
V2:
make have_callable_console() available for !PRINTK configs (lkp@...el.com)

 kernel/printk/printk.c | 101 +++++++++++++++++++++++++------------------------
 1 file changed, 51 insertions(+), 50 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 7ebcfea..ae641d7 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1452,9 +1452,6 @@ static void call_console_drivers(int level,
 			continue;
 		if (!con->write)
 			continue;
-		if (!cpu_online(smp_processor_id()) &&
-		    !(con->flags & CON_ANYTIME))
-			continue;
 		if (con->flags & CON_EXTENDED)
 			con->write(con, ext_text, ext_len);
 		else
@@ -1485,33 +1482,6 @@ static void zap_locks(void)
 }
 
 /*
- * Check if we have any console that is capable of printing while cpu is
- * booting or shutting down. Requires console_sem.
- */
-static int have_callable_console(void)
-{
-	struct console *con;
-
-	for_each_console(con)
-		if (con->flags & CON_ANYTIME)
-			return 1;
-
-	return 0;
-}
-
-/*
- * Can we actually use the console at this time on this cpu?
- *
- * Console drivers may assume that per-cpu resources have been allocated. So
- * unless they're explicitly marked as being able to cope (CON_ANYTIME) don't
- * call them until this CPU is officially up.
- */
-static inline int can_use_console(unsigned int cpu)
-{
-	return cpu_online(cpu) || have_callable_console();
-}
-
-/*
  * Try to get console ownership to actually show the kernel
  * messages from a 'printk'. Return true (and with the
  * console_lock held, and 'console_locked' set) if it
@@ -1519,21 +1489,7 @@ static inline int can_use_console(unsigned int cpu)
  */
 static int console_trylock_for_printk(void)
 {
-	unsigned int cpu = smp_processor_id();
-
-	if (!console_trylock())
-		return 0;
-	/*
-	 * If we can't use the console, we need to release the console
-	 * semaphore by hand to avoid flushing the buffer. We need to hold the
-	 * console semaphore in order to do this test safely.
-	 */
-	if (!can_use_console(cpu)) {
-		console_locked = 0;
-		up_console_sem();
-		return 0;
-	}
-	return 1;
+	return console_trylock();
 }
 
 int printk_delay_msec __read_mostly;
@@ -2177,10 +2133,38 @@ int is_console_locked(void)
 	return console_locked;
 }
 
-static void console_cont_flush(char *text, size_t size)
+/*
+ * Check if we have any console that is capable of printing while cpu is
+ * booting or shutting down. Requires console_sem.
+ */
+static int have_callable_console(void)
+{
+	struct console *con;
+
+	for_each_console(con)
+		if (con->flags & CON_ANYTIME)
+			return 1;
+
+	return 0;
+}
+
+/*
+ * Can we actually use the console at this time on this cpu?
+ *
+ * Console drivers may assume that per-cpu resources have been allocated. So
+ * unless they're explicitly marked as being able to cope (CON_ANYTIME) don't
+ * call them until this CPU is officially up.
+ */
+static inline int can_use_console(void)
+{
+	return cpu_online(smp_processor_id()) || have_callable_console();
+}
+
+static bool console_cont_flush(char *text, size_t size)
 {
 	unsigned long flags;
 	size_t len;
+	bool ret = true;
 
 	raw_spin_lock_irqsave(&logbuf_lock, flags);
 
@@ -2195,15 +2179,21 @@ static void console_cont_flush(char *text, size_t size)
 	if (console_seq < log_next_seq && !cont.cons)
 		goto out;
 
+	if (!can_use_console()) {
+		ret = false;
+		goto out;
+	}
+
 	len = cont_print_text(text, size);
 	raw_spin_unlock(&logbuf_lock);
 	stop_critical_timings();
 	call_console_drivers(cont.level, NULL, 0, text, len);
 	start_critical_timings();
 	local_irq_restore(flags);
-	return;
+	return ret;
 out:
 	raw_spin_unlock_irqrestore(&logbuf_lock, flags);
+	return ret;
 }
 
 /**
@@ -2227,7 +2217,7 @@ void console_unlock(void)
 	static u64 seen_seq;
 	unsigned long flags;
 	bool wake_klogd = false;
-	bool do_cond_resched, retry;
+	bool do_cond_resched, retry, aborted = false;
 
 	if (console_suspended) {
 		up_console_sem();
@@ -2248,7 +2238,12 @@ void console_unlock(void)
 	console_may_schedule = 0;
 
 	/* flush buffered message fragment immediately to console */
-	console_cont_flush(text, sizeof(text));
+	if (!console_cont_flush(text, sizeof(text))) {
+		console_locked = 0;
+		up_console_sem();
+		return;
+	}
+
 again:
 	for (;;) {
 		struct printk_log *msg;
@@ -2257,6 +2252,12 @@ again:
 		int level;
 
 		raw_spin_lock_irqsave(&logbuf_lock, flags);
+
+		if (!can_use_console()) {
+			aborted = true;
+			break;
+		}
+
 		if (seen_seq != log_next_seq) {
 			wake_klogd = true;
 			seen_seq = log_next_seq;
@@ -2337,7 +2338,7 @@ skip:
 	 * flush, no worries.
 	 */
 	raw_spin_lock(&logbuf_lock);
-	retry = console_seq != log_next_seq;
+	retry = !aborted && (console_seq != log_next_seq);
 	raw_spin_unlock_irqrestore(&logbuf_lock, flags);
 
 	if (retry && console_trylock())
-- 
2.7.0

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ