From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Fastabend Subject: [net-next PATCH] net: ptr_ring: otherwise safe empty checks can overrun array bounds Date: Wed, 27 Dec 2017 19:50:25 -0800 Message-ID: <20171228035024.14699.69453.stgit@john-Precision-Tower-5810> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: xiyou.wangcong@gmail.com, jiri@resnulli.us, netdev@vger.kernel.org To: jakub.kicinski@netronome.com, davem@davemloft.net, mst@redhat.com Return-path: Received: from mail-pl0-f68.google.com ([209.85.160.68]:33311 "EHLO mail-pl0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753061AbdL1Dui (ORCPT ); Wed, 27 Dec 2017 22:50:38 -0500 Received: by mail-pl0-f68.google.com with SMTP id 1so19039335plv.0 for ; Wed, 27 Dec 2017 19:50:38 -0800 (PST) Sender: netdev-owner@vger.kernel.org List-ID: When running consumer and/or producer operations and empty checks in parallel its possible to have the empty check run past the end of the array. The scenario occurs when an empty check is run while __ptr_ring_discard_one() is in progress. Specifically after the consumer_head is incremented but before (consumer_head >= ring_size) check is made and the consumer head is zeroe'd. To resolve this, without having to rework how consumer/producer ops work on the array, simply add an extra dummy slot to the end of the array. Even if we did a rework to avoid the extra slot it looks like the normal case checks would suffer some so best to just allocate an extra pointer. Reported-by: Jakub Kicinski Fixes: c5ad119fb6c09 ("net: sched: pfifo_fast use skb_array") Signed-off-by: John Fastabend --- include/linux/ptr_ring.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h index 6866df4..13fb06a 100644 --- a/include/linux/ptr_ring.h +++ b/include/linux/ptr_ring.h @@ -447,7 +447,12 @@ static inline int ptr_ring_consume_batched_bh(struct ptr_ring *r, static inline void **__ptr_ring_init_queue_alloc(unsigned int size, gfp_t gfp) { - return kcalloc(size, sizeof(void *), gfp); + /* Allocate an extra dummy element at end of ring to avoid consumer head + * or produce head access past the end of the array. Possible when + * producer/consumer operations and __ptr_ring_peek operations run in + * parallel. + */ + return kcalloc(size + 1, sizeof(void *), gfp); } static inline void __ptr_ring_set_size(struct ptr_ring *r, int size)