linux-kernel - Re: [PATCH tip/core/locking 4/4] Documentation/memory-barriers.txt: Document ACCESS

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, 5 Dec 2013 11:11:37 +0100
From:	Ingo Molnar <mingo@...nel.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org, laijs@...fujitsu.com,
	dipankar@...ibm.com, akpm@...ux-foundation.org,
	josh@...htriplett.org, niv@...ibm.com, tglx@...utronix.de,
	peterz@...radead.org, rostedt@...dmis.org, dhowells@...hat.com,
	edumazet@...gle.com, darren@...art.com, fweisbec@...il.com,
	sbw@....edu, Oleg Nesterov <oleg@...hat.com>,
	Jonathan Corbet <corbet@....net>,
	Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [PATCH tip/core/locking 4/4] Documentation/memory-barriers.txt:
 Document ACCESS_ONCE()


* Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:

> ----- Original Message -----
> > From: "Ingo Molnar" <mingo@...nel.org>
> > To: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
> > Cc: linux-kernel@...r.kernel.org, laijs@...fujitsu.com, dipankar@...ibm.com, akpm@...ux-foundation.org, "mathieu
> > desnoyers" <mathieu.desnoyers@...icios.com>, josh@...htriplett.org, niv@...ibm.com, tglx@...utronix.de,
> > peterz@...radead.org, rostedt@...dmis.org, dhowells@...hat.com, edumazet@...gle.com, darren@...art.com,
> > fweisbec@...il.com, sbw@....edu, "Oleg Nesterov" <oleg@...hat.com>, "Jonathan Corbet" <corbet@....net>, "Rusty
> > Russell" <rusty@...tcorp.com.au>
> > Sent: Thursday, December 5, 2013 10:33:34 AM
> > Subject: Re: [PATCH tip/core/locking 4/4] Documentation/memory-barriers.txt: Document ACCESS_ONCE()
> > 
> > 
> > * Paul E. McKenney <paulmck@...ux.vnet.ibm.com> wrote:
> > 
> > > + (*) The compiler is within its rights to reorder memory accesses unless
> > > +     you tell it not to.  For example, consider the following interaction
> > > +     between process-level code and an interrupt handler:
> > > +
> > > +	void process_level(void)
> > > +	{
> > > +		msg = get_message();
> > > +		flag = true;
> > > +	}
> > > +
> > > +	void interrupt_handler(void)
> > > +	{
> > > +		if (flag)
> > > +			process_message(msg);
> > > +	}
> > > +
> > > +     There is nothing to prevent the the compiler from transforming
> > > +     process_level() to the following, in fact, this might well be a
> > > +     win for single-threaded code:
> > > +
> > > +	void process_level(void)
> > > +	{
> > > +		flag = true;
> > > +		msg = get_message();
> > > +	}
> > > +
> > > +     If the interrupt occurs between these two statement, then
> > > +     interrupt_handler() might be passed a garbled msg.  Use ACCESS_ONCE()
> > > +     to prevent this as follows:
> > > +
> > > +	void process_level(void)
> > > +	{
> > > +		ACCESS_ONCE(msg) = get_message();
> > > +		ACCESS_ONCE(flag) = true;
> > > +	}
> > > +
> > > +	void interrupt_handler(void)
> > > +	{
> > > +		if (ACCESS_ONCE(flag))
> > > +			process_message(ACCESS_ONCE(msg));
> > > +	}
> > 
> > Technically, if the interrupt handler is the innermost context, the
> > ACCESS_ONCE() is not needed in the interrupt_handler() code.
> > 
> > Since for the vast majority of Linux code IRQ handlers are the most
> > atomic contexts (very few drivers deal with NMIs) I suspect we should
> > either remove that ACCESS_ONCE() from the example or add a comment
> > explaining that in many cases those are superfluous?
> 
> It might be worthwhile to state clearly those two different use-cases:
> 
> * no nesting (e.g. process vs process), where both sides of the access
>   need ACCESS_ONCE().
> 
> * nested access: the outer context needs ACCESS_ONCE(), but not the
>   inner context.
> 
> We don't actually care about IRQs being the "most atomic" context, 
> and we should not care about NMIs in this example. Only the relative 
> nesting of the execution contexts accessing the data matter.

I meant 'most atomic' in the sense of no NMI context having access to 
those same shared variables, obviously.

We do actually have 'triple' nesting in Linux, NMI context code with 
non-trivial complexity that accesses variables in different code paths 
from IRQ codepaths and from process level code paths, so technically 
the original example is valid - I just think misleading to most 
readers.

Thanks,

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