linux-kernel - Re: [PATCH] time: Add locking to xtime access in get

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, 05 May 2011 13:17:01 -0700
From:	john stultz <johnstul@...ibm.com>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	lkml <linux-kernel@...r.kernel.org>,
	Paul Mackerras <paulus@...ba.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Anton Blanchard <anton@...ba.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] time: Add locking to xtime access in get_seconds()

On Thu, 2011-05-05 at 19:57 +0200, Andi Kleen wrote:
> > I suspect the reason this hasn't been triggered on x86 or power6 is due
> > to compiler or processor optimizations reordering the assignment to in
> > effect make it atomic. Or maybe the timing window to see the issue is
> > harder to observe?
> 
> On x86 all aligned stores are atomic. So I don't see how this 
> could be a problem ever.

No no. The issue was with the fact that in update_xtime_cache we modify
xtime_cache twice (once setting it possibly backwards to xtime, then
adding in the nsec offset).

Since get_seconds does no locking, this issue should be visible
anywhere, as long as you manage to hit the race window between the first
assignment and the second.

However, in the testing, the issue only showed up on P7, but not P6 or
x86.

My guess was that the code:

xtime_cache.sec = xtime.sec
xtime_cache.nsec = xtime.nsec
xtime_cache.sec = xtime_cache.sec 
		 + div(xtime_cache.nsec + nsec, NSEC_PER_SEC, &rem);
xtime_cache.nsec = rem

Was getting rearranged to:

xtime_cache.sec = xtime.sec 
		+ div(xtime.nsec + nsec, NSEC_PER_SEC, &rem);
xtime_cache.nsec = rem


Which makes the xtime_cache.sec update atomic.

But its just a guess.

thanks
-john


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