netdev - Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier

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:   Tue, 5 Dec 2017 20:57:46 +0200
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org, mingo@...nel.org,
        jiangshanlai@...il.com, dipankar@...ibm.com,
        akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
        josh@...htriplett.org, tglx@...utronix.de, rostedt@...dmis.org,
        dhowells@...hat.com, edumazet@...gle.com, fweisbec@...il.com,
        oleg@...hat.com, Jason Wang <jasowang@...hat.com>,
        kvm@...r.kernel.org, virtualization@...ts.linux-foundation.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant
 read_barrier_depends()

On Tue, Dec 05, 2017 at 07:39:46PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 08:31:20PM +0200, Michael S. Tsirkin wrote:
> 
> > Apropos, READ_ONCE is now asymmetrical with WRITE_ONCE.
> > 
> > I can read a pointer with READ_ONCE and be sure the value
> > is sane, but only if I also remember to put in smp_wmb before
> > WRITE_ONCE. Otherwise the pointer is ok but no guarantees
> > about the data pointed to.
> 
> That was already the case on everything except Alpha. And the canonical
> match do the data dependency is store_release, not wmb.

Oh, interesting

static __always_inline void __write_once_size(volatile void *p, void *res, int size)
{
        switch (size) {
        case 1: *(volatile __u8 *)p = *(__u8 *)res; break;
        case 2: *(volatile __u16 *)p = *(__u16 *)res; break;
        case 4: *(volatile __u32 *)p = *(__u32 *)res; break;
        case 8: *(volatile __u64 *)p = *(__u64 *)res; break;
        default:
                barrier();
                __builtin_memcpy((void *)p, (const void *)res, size);
                barrier();
        }
}

#define WRITE_ONCE(x, val) \
({                                                      \
        union { typeof(x) __val; char __c[1]; } __u =   \
                { .__val = (__force typeof(x)) (val) }; \
        __write_once_size(&(x), __u.__c, sizeof(x));    \
        __u.__val;                                      \
})

I don't see WRITE_ONCE inserting any barriers, release or
write.

So it seems that on an architecture where writes can be reordered,
if I do

*pointer = 0xa;
WRITE_ONCE(array[x], pointer);

array write might bypass the pointer write,
and readers will read a stale value.




-- 
MST