linux-kernel - Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC

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] [day] [month] [year] [list]

Date:   Mon, 8 Jan 2018 19:30:21 +0100
From:   Andrea Arcangeli <aarcange@...hat.com>
To:     Alexey Dobriyan <adobriyan@...il.com>
Cc:     tim.c.chen@...ux.intel.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable
 SPEC_CTRL feature

On Mon, Jan 08, 2018 at 08:43:40PM +0300, Alexey Dobriyan wrote:
> > +	len = sprintf(buf, "%d\n", READ_ONCE(*field));
> 
> READ_ONCE isn't necessary as there is only one read being made.

Others might disagree but you shouldn't ever let gcc touch any memory
that can change under gcc without READ_ONCE or volatile.

Ages ago I was told in a switch() statement gcc could decide to use an
hash and re-read the value and crash.

Not for the simple case above, and we often don't use READ_ONCE and we
might user barrier() instead, but it would be better to use READ_ONCE.

READ_ONCE is more correct and there's no point to try to optimize it
especially if there's only one read being made in that function.

Either version in practice will work, but READ_ONCE is preferable IMHO.

Thanks,
Andrea