From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH net-next 05/12] ptr_ring: disallow lockless __ptr_ring_full Date: Fri, 26 Jan 2018 04:46:12 +0200 Message-ID: <20180126044421-mutt-send-email-mst@kernel.org> References: <1516923320-16959-1-git-send-email-mst@redhat.com> <1516923320-16959-6-git-send-email-mst@redhat.com> <4bd89514-062c-dadf-53db-e6fac59512b1@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, John Fastabend , David Miller To: Jason Wang Return-path: Content-Disposition: inline In-Reply-To: <4bd89514-062c-dadf-53db-e6fac59512b1@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Fri, Jan 26, 2018 at 10:38:05AM +0800, Jason Wang wrote: > > > On 2018年01月26日 07:36, Michael S. Tsirkin wrote: > > Similar to bcecb4bbf88a ("net: ptr_ring: otherwise safe empty checks can > > overrun array bounds") a lockless use of __ptr_ring_full might > > cause an out of bounds access. > > > > We can fix this, but it's easier to just disallow lockless > > __ptr_ring_full for now. > > It looks to me that just fix this is better than disallow through doc (which > is easily to be ignored ...). > > Thanks lockless is tricky, and I'd rather not sprinkle READ/WRITE_ONCE where they aren't necessary. > > > > Signed-off-by: Michael S. Tsirkin > > --- > > include/linux/ptr_ring.h | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h > > index 9a72d8f..f175846 100644 > > --- a/include/linux/ptr_ring.h > > +++ b/include/linux/ptr_ring.h > > @@ -45,9 +45,10 @@ struct ptr_ring { > > }; > > /* Note: callers invoking this in a loop must use a compiler barrier, > > - * for example cpu_relax(). If ring is ever resized, callers must hold > > - * producer_lock - see e.g. ptr_ring_full. Otherwise, if callers don't hold > > - * producer_lock, the next call to __ptr_ring_produce may fail. > > + * for example cpu_relax(). > > + * > > + * NB: this is unlike __ptr_ring_empty in that callers must hold producer_lock: > > + * see e.g. ptr_ring_full. > > */ > > static inline bool __ptr_ring_full(struct ptr_ring *r) > > {