All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: jakub.kicinski@netronome.com, davem@davemloft.net, mst@redhat.com
Cc: xiyou.wangcong@gmail.com, jiri@resnulli.us, netdev@vger.kernel.org
Subject: [net-next PATCH] net: ptr_ring: otherwise safe empty checks can overrun array bounds
Date: Wed, 27 Dec 2017 19:50:25 -0800	[thread overview]
Message-ID: <20171228035024.14699.69453.stgit@john-Precision-Tower-5810> (raw)

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 <jakub.kicinski@netronome.com>
Fixes: c5ad119fb6c09 ("net: sched: pfifo_fast use skb_array")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 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)

             reply	other threads:[~2017-12-28  3:50 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-28  3:50 John Fastabend [this message]
2018-01-02 16:52 ` [net-next PATCH] net: ptr_ring: otherwise safe empty checks can overrun array bounds David Miller
2018-01-02 17:01   ` Michael S. Tsirkin
2018-01-02 17:17     ` Michael S. Tsirkin
2018-01-02 21:27       ` John Fastabend
2018-01-02 23:12         ` Michael S. Tsirkin
2018-01-03  0:25           ` John Fastabend
2018-01-03 15:50             ` Michael S. Tsirkin
2018-01-03 17:46               ` John Fastabend
2018-01-03 18:34                 ` Michael S. Tsirkin
2018-01-02 18:33     ` David Miller
2018-01-02 16:53 ` Michael S. Tsirkin
2018-01-02 17:01   ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171228035024.14699.69453.stgit@john-Precision-Tower-5810 \
    --to=john.fastabend@gmail.com \
    --cc=davem@davemloft.net \
    --cc=jakub.kicinski@netronome.com \
    --cc=jiri@resnulli.us \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.