linux-kernel - Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer

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]

Message-ID: <20190531083542.GL2623@hirez.programming.kicks-ass.net>
Date:   Fri, 31 May 2019 10:35:42 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Andrea Parri <andrea.parri@...rulasolutions.com>
Cc:     Greg KH <gregkh@...uxfoundation.org>,
        David Howells <dhowells@...hat.com>, viro@...iv.linux.org.uk,
        raven@...maw.net, linux-fsdevel@...r.kernel.org,
        linux-api@...r.kernel.org, linux-block@...r.kernel.org,
        keyrings@...r.kernel.org, linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org, Will Deacon <will.deacon@....com>,
        "Paul E. McKenney" <paulmck@...ux.ibm.com>,
        Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH 1/7] General notification queue with user mmap()'able
 ring buffer

On Thu, May 30, 2019 at 11:50:39AM +0200, Andrea Parri wrote:
> > > Looking at the perf ring buffer, there appears to be a missing barrier in
> > > perf_aux_output_end():
> > > 
> > > 	rb->user_page->aux_head = rb->aux_head;
> > > 
> > > should be:
> > > 
> > > 	smp_store_release(&rb->user_page->aux_head, rb->aux_head);
> > > 
> > > It should also be using smp_load_acquire().  See
> > > Documentation/core-api/circular-buffers.rst
> > > 
> > > And a (partial) patch has been proposed: https://lkml.org/lkml/2018/5/10/249
> > 
> > So, if that's all that needs to be fixed, can you use the same
> > buffer/code if that patch is merged?
> 
> That's about one year old...: let me add the usual suspects in Cc:  ;-)
> since I'm not sure what the plan was (or if I'm missing something) ...

The AUX crud is 'special' and smp_store_release() doesn't really help in
many cases. Notable, AUX is typically used in combination with a
hardware writer. The driver is in charge of odering here, the generic
code doesn't know what the appropriate barrier (if any) is and would
have to resort to the most expensive/heavy one available.

Also see the comment right above this function:

 "It is the
  pmu driver's responsibility to observe ordering rules of the hardware,
  so that all the data is externally visible before this is called."