linux-kernel - Re: [patch 1/3] printk: fix wake_up

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: <20101126124247.GC7023@osiris.boeblingen.de.ibm.com>
Date:	Fri, 26 Nov 2010 13:42:47 +0100
From:	Heiko Carstens <heiko.carstens@...ibm.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	linux-kernel@...r.kernel.org,
	Christof Schmitt <christof.schmitt@...ibm.com>,
	Frank Blaschka <frank.blaschka@...ibm.com>,
	Horst Hartmann <horsth@...ux.vnet.ibm.com>
Subject: Re: [patch 1/3] printk: fix wake_up_klogd() vs cpu hotplug

On Fri, Nov 26, 2010 at 01:35:09PM +0100, Eric Dumazet wrote:
> Le vendredi 26 novembre 2010 à 13:00 +0100, Heiko Carstens a écrit :
> >  void wake_up_klogd(void)
> >  {
> > -	if (waitqueue_active(&log_wait))
> > -		__raw_get_cpu_var(printk_pending) = 1;
> > +	if (waitqueue_active(&log_wait)) {
> > +		get_cpu_var(printk_pending) = 1;
> > +		put_cpu_var(printk_pending);
> > +	}
> >  }
> 
> Please use :
> 
> this_cpu_write(printk_pending, 1);
> 
> It is faster on x86, and does the right thing too.

Ah, right. I wasn't aware that such a thing even exists.
Updated patch below:

Subject: [PATCH] printk: fix wake_up_klogd() vs cpu hotplug

From: Heiko Carstens <heiko.carstens@...ibm.com>

wake_up_klogd() may get called from preemtible context but uses
__raw_get_cpu_var() to write to a per cpu variable. If it gets preempted between
getting the address and writing to it, the cpu in question could be offline if
the process gets scheduled back and hence writes to the per cpu data of an offline
cpu.

No idea why that behaviour was introduced with fa33507a "printk: robustify
printk, fix #2" which was supposed to fix a "using smp_processor_id() in
preemptible" warning.

Let's use this_cpu_write() instead which disables preemption and makes sure that
the outlined scenario cannot happen.

Signed-off-by: Heiko Carstens <heiko.carstens@...ibm.com>
---
 kernel/printk.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1088,7 +1088,7 @@ int printk_needs_cpu(int cpu)
 void wake_up_klogd(void)
 {
 	if (waitqueue_active(&log_wait))
-		__raw_get_cpu_var(printk_pending) = 1;
+		this_cpu_write(printk_pending, 1);
 }
 
 /**
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/