linux-kernel - Re: [PATCH] ARCv2: Implement atomic64 based on LLOCKD/SCONDD instructions

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, 8 Sep 2016 15:24:10 -0700
From:   Vineet Gupta <Vineet.Gupta1@...opsys.com>
To:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Peter Zijlstra" <peterz@...radead.org>
CC:     Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
        Will Deacon <will.deacon@....com>,
        "linux-snps-arc@...ts.infradead.org" 
        <linux-snps-arc@...ts.infradead.org>
Subject: Re: [PATCH] ARCv2: Implement atomic64 based on LLOCKD/SCONDD
 instructions

On 09/08/2016 12:29 PM, Vineet Gupta wrote:
> One thing I'm not sure of is the lack of explicit memory clobber in
> barrier-less ops e.g. atomic64_add() (BTW same is true for 32-bit
> atomic_add() as well). Per commit 398aa66827 ("ARM: 6212/1: atomic ops:
> add memory constraints to inline asm ") Will fixed ARM code by adding
> appropriate constraint to atomic64_add(). For ARC instead adding memory
> clobber to atomic64_set() does the trick (otherwise self-test is broked)
> This is on ARC we can't possibly use "m" in atomic64_add() since that make gcc
> emit register relative effective addresses which LLOCKD/SCONDD are not
> allowed by ISA

So interestingly my self-test run fine, but I had this oldish version stashed
somewhere which did something liek below and that clearly generates wrong code.

int my_test_atomic(void)
{
    long v0 = 0x33333333;
    long onestwos = 0x11112222;

    atomic_t v = ATOMIC_INIT(v0);
    long long r = v0;
    int ret = 0;

    atomic_set(&v, v0); r = v0;
    atomic_add(onestwos, &v);
    r += onestwos;
    if (v.counter != r) {      /* <------ */
                ret = 3;   /* error */
    }

    return ret;
}

key here is the check - if we access the atomic directly, I get error. If I use
atomic_read() which forces a reload due to volatile, things are hunky dory. So it
seems to me we don't need memory clobber or equivalent in barrier less atomics
except the set. Seems too fragile ?

-Vineet