linux-kernel - Re: [RFC PATCH] printk: Use ACCESS_ONCE() instead of a volatile type

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:	Fri, 14 Nov 2014 00:24:36 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Alex Elder <elder@...aro.org>
Cc:	Pranith Kumar <bobby.prani@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Petr Mladek <pmladek@...e.cz>, Jan Kara <jack@...e.cz>,
	"Luis R. Rodriguez" <mcgrof@...e.com>,
	Joe Perches <joe@...ches.com>,
	open list <linux-kernel@...r.kernel.org>,
	paulmck@...ux.vnet.ibm.com
Subject: Re: [RFC PATCH] printk: Use ACCESS_ONCE() instead of a volatile
 type

On Thu, 13 Nov 2014 23:57:22 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:

> That assignment is what it is initialized to at boot up. I can't see
> any optimization that would cause gcc to modify that. Especially since
> we are hiding its accesses within the ACCESS_ONCE(). That alone should
> confuse gcc enough to leave it a hell alone J.

I'm actually wondering if the ACCESS_ONCE or volatile is even needed.

static variables are used to maintain state, and that goes for
recursive functions. gcc should not touch it.

Now perhaps it can see that there is no recursion for logbuf_cpu to be
set to the current cpu (which would be interesting since the
smp_processor_id() call is also hidden from gcc), and it might optimize
it out. But that would not protect us from NMIs doing a printk().
Although this code doesn't protect us from that anyway if an NMI were
to come in right after taking the logbuf_lock and before setting
logbuf_cpu. In that case, logbuf_cpu will not be set to this_cpu and a
deadlock can still occur. This code only makes the race window smaller.

I'm thinking the correct change is to nuke all of it. Perhaps the only
reason using volatile here was not a bug is because volatile wasn't
needed in the first place!

-- Steve
--
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/