linux-kernel - Re: sequence lock in Linux

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: <20100611213602.GI2394@linux.vnet.ibm.com>
Date:	Fri, 11 Jun 2010 14:36:02 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	"H. Peter Anvin" <hpa@...or.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	linux-kernel@...r.kernel.org
Subject: Re: sequence lock in Linux

On Fri, Jun 11, 2010 at 02:06:01PM -0700, H. Peter Anvin wrote:
> On 06/11/2010 01:36 PM, Paul E. McKenney wrote:
> > 
> > The reason that the C standard permits this is to allow for things like
> > 8-bit CPUs, which are simply unable to load or store 32-bit quantities
> > except by doing it chunkwise.  But I don't expect the Linux kernel to
> > boot on these, and certainly not on any of the ones that I have used!
> > 
> > I most definitely remember seeing a gcc guarantee that loads and stores
> > would be done in one instruction whenever the hardware supported this,
> > but I am not finding it today.  :-(
> 
> What gcc does not -- and should not -- guarantee is that accessing a
> non-volatile member is done exactly once.  As Mathieu pointed out, it
> can choose to drop it due to register pressure and load it again.
> 
> What is possibly a much bigger risk -- since this is an inline -- is
> that the value is cached from a previous piece of code, *or* that since
> the structure is const(!) that the second read in the repeat loop is
> elided.  Presumably current versions of gcc don't do that across a
> memory clobber, but that doesn't seem entirely out of the question.

Memory barriers in the sequence-lock code prevent this, assuming, as
you point out, that memory clobber works (but if it doesn't, it should
be fixed):

o	write_seqlock() and write_tryseqlock() each have an smp_wmb()
	following the increment.  Ditto for write_seqcount_begin().

o	write_sequnlock() has an smp_wmb() preceding the increment,
	and ditto for write_seqcount_end().  There are thus two smp_wmb()
	calls between the increments in the usual code sequence:

		write_seqlock(&l);
		do_something();
		write_sequnlock();

o	read_seqbegin() has an smp_rmb() following its read from
	->sequence.  Ditto for read_seqcount_begin().

o	read_seqretry() has an smp_rmb() preceding its read from
	->sequence, and ditto for read_seqcount_retry().  There are thus
	two smp_wmb() calls between the reads in the usual code sequence:

		do {
			s = read_seqbegin(&l);
			read_something();
		} while read_seqretry(&l, s);

So sequence locks should be pretty safe, at least as far as this
vulnerability is concerned. ;-)

							Thanx, Paul
--
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/