linux-kernel - Re: [patch 54/55] timekeeping: Provide fast and NMI safe access to CLOCK_MONOTONIC[

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:	Sat, 12 Jul 2014 21:28:48 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
cc:	LKML <linux-kernel@...r.kernel.org>,
	John Stultz <john.stultz@...aro.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [patch 54/55] timekeeping: Provide fast and NMI safe access to
 CLOCK_MONOTONIC[_RAW]

On Sat, 12 Jul 2014, Mathieu Desnoyers wrote:
> I'm perhaps missing something here, but what happens with the
> following scenario ?
> 
> Initial conditions:
> 
> tkf->seq = 0
> tkf->base[0] and tkf->base[1] are initialized.
> 
> CPU 0                                      CPU 1
> ------------                               ----------------
> update:
>   tkf->seq++
>   smb_wmb()
>   tkf->seq++ (reordered before update)
>                                            reader:
>                                            seq = tkf->seq (reads 2)
>                                            smp_rmb()
>                                            idx = seq & 0x01
>                                            now = now(tkf->base[idx]   (reads base[0])
>   update(tkf->base[0], tk) (racy concurrent update)
>                                            smp_rmb()
>                                            while (seq != tkf->seq) (they are equal)
> 
> So AFAIU, we end up returning a corrupted value. Adding a
> smp_wmb() between update of base[0] and increment of seq,
> as well as between update of base[1] and the _following_
> increment of seq (next update call) would fix this.
> 
> Thoughts ?

Well, the actual implementation does:

+       /* Force readers off to base[1] */
+       raw_write_seqcount_begin(&tkf->seq);
+
+       /* Update base[0] */
+       base->clock = clk;
+       base->cycle_last = clk->cycle_last;
+       base->base = tbase;
+       base->shift = shift;
+       base->mult = mult;
+
+       /* Force readers back to base[0] */
+       raw_write_seqcount_end(&tkf->seq);
+
+       /* Update base[1] */
+       base++;
+       base->clock = clk;
+       base->cycle_last = clk->cycle_last;
+       base->base = tbase;
+       base->shift = shift;
+       base->mult = mult;

Where raw_write_seqcount_begin/raw_write_seqcount_end provides the
required memory barriers.

Sure I should update the documentaion ....

Thanks,

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