linux-kernel - Re: [PATCH] ptr_ring: Add barriers to fix NULL-pointer exception

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

Message-ID: <20171206175023-mutt-send-email-mst@kernel.org>
Date:   Wed, 6 Dec 2017 17:57:25 +0200
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     "Cherian, George" <George.Cherian@...ium.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>
Subject: Re: [PATCH] ptr_ring: Add barriers to fix NULL-pointer exception

On Wed, Dec 06, 2017 at 02:08:54PM +0000, Cherian, George wrote:
> > @@ -275,6 +281,13 @@ static inline void *__ptr_ring_consume(struct ptr_ring
> *r)
> > if (ptr)
> > __ptr_ring_discard_one(r);
> >
> > + /*
> > + * This barrier is necessary in order to prevent race condition with
> > + * with __ptr_ring_produce(). Make sure all the elements of ptr is
> > + * in sync with the earlier writes which was done prior to pushing
> > + * it to ring
> > + */
> > + rmb();
> > return ptr;
> > }
> 
> You are trying to synchronise two CPUs so non-smp barriers make no
> sense. wmb/rmb are for synchronising with MMIO.
> 
> What happens when CONFIG_SMP is not set. smp_wmb/rmb becomes compiler barriers
> (atleast for arm64).

And that is because all read and writes always appear in order when done
from the same CPU.

In case of reads, we do not need a barrier at all (except on dec alpha)
because a read through a pointer can't bypass a read of a pointer.

> I guess that is not what we need.

Maybe, but I don't yet see why not.

> An SMP barrier cannot
> replace a mandatory barrier, but a mandatory barrier can replace an SMP
> barrier.

This does imply that you can always replace a weak barrier with a strong
one, but does not mean you should.

> I will try out your patch too and update the results.
> But I would need couple of days time. Sorry for the delay.

Thanks for the testing.

-- 
MST