From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [Patch net] ptr_ring: wrap back ->producer in __ptr_ring_swap_queue() Date: Sun, 30 Dec 2018 19:52:35 -0500 Message-ID: <20181230194355-mutt-send-email-mst@kernel.org> References: <20181230204342.22699-1-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, John Fastabend , Jason Wang To: Cong Wang Return-path: Received: from mx1.redhat.com ([209.132.183.28]:38126 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725938AbeLaAwh (ORCPT ); Sun, 30 Dec 2018 19:52:37 -0500 Content-Disposition: inline In-Reply-To: <20181230204342.22699-1-xiyou.wangcong@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Sun, Dec 30, 2018 at 12:43:42PM -0800, Cong Wang wrote: > __ptr_ring_swap_queue() tries to move pointers from the old > ring to the new one, but it forgets to check if ->producer > is beyond the new size at the end of the operation. This leads > to an out-of-bound access in __ptr_ring_produce() as reported > by syzbot. > > Reported-by: syzbot+8993c0fa96d57c399735@syzkaller.appspotmail.com > Fixes: 5d49de532002 ("ptr_ring: resize support") > Cc: "Michael S. Tsirkin" > Cc: John Fastabend > Cc: Jason Wang > Signed-off-by: Cong Wang > --- > include/linux/ptr_ring.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h > index 6894976b54e3..186cd8e970c7 100644 > --- a/include/linux/ptr_ring.h > +++ b/include/linux/ptr_ring.h > @@ -573,6 +573,8 @@ static inline void **__ptr_ring_swap_queue(struct ptr_ring *r, void **queue, > else if (destroy) > destroy(ptr); > > + if (producer >= size) > + producer = 0; > __ptr_ring_set_size(r, size); > r->producer = producer; > r->consumer_head = 0; > -- > 2.19.2 To clarify the commit log a little bit: if (producer < size) queue[producer++] = ptr; if the new size is smaller than the old one, we producer can end up being equal to the new size thus pointing one beyond the end of the array. So when we allocated one extra entry it was fine, thus maybe we should rather list: Fixes: 9fb582b67072 ("Revert "net: ptr_ring: otherwise safe empty checks can overrun array bounds"") The patch itself is good though. So Acked-by: Michael S. Tsirkin and pls queue this for stable. -- MST