Re: [Patch net] ptr_ring: wrap back ->producer in __ptr_ring_swap_queue()

["Michael S. Tsirkin" <mst@redhat.com>]

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" <mst@redhat.com>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  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 <mst@redhat.com>

and pls queue this for stable.

-- 
MST