Re: 答复: [PATCH] net/virtio-user: fix overflow

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Tue, Mar 14, 2017 at 04:20:39PM +0800, Wenfeng Liu wrote:
> Hi Yuanhan,
> 
> >On Mon, Mar 13, 2017 at 03:09:15PM +0000, Wenfeng Liu wrote:
> >> This commit fixes an array overflow when number of queues is higher than
> 8.
> >
> >Firstly, this commit log could be a bit more informative, to something
> >like:
> >
> >    virtio-user limits the qeueue number to 8 but provides no limit
> >    check against the queue number input from user. If a bigger queue
> >    number (> 8) is given, there is an overflow issue. Doing a sanity
> >    check could avoid it.
> >
> 
> Sure, I will revise the commit log accordingly.
> 
> >> 
> >> Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
> >> 
> >> Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>
> >> ---
> >>  drivers/net/virtio/virtio_pci.h                  | 3 ++-
> >>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +-  
> >> drivers/net/virtio/virtio_user/virtio_user_dev.h | 6 +++---
> >>  drivers/net/virtio/virtio_user_ethdev.c          | 7 +++++++
> >>  4 files changed, 13 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/drivers/net/virtio/virtio_pci.h 
> >> b/drivers/net/virtio/virtio_pci.h index 59e45c4..bd940b4 100644
> >> --- a/drivers/net/virtio/virtio_pci.h
> >> +++ b/drivers/net/virtio/virtio_pci.h
> >> @@ -160,7 +160,8 @@
> >>  /*
> >>   * Maximum number of virtqueues per device.
> >>   */
> >> -#define VIRTIO_MAX_VIRTQUEUES 8
> >> +#define VIRTIO_MAX_VIRTQUEUE_PAIRS 8
> >> +#define VIRTIO_MAX_VIRTQUEUES VIRTIO_MAX_VIRTQUEUE_PAIRS * 2 + 1
> >>  
> >>  /* Common configuration */
> >>  #define VIRTIO_PCI_CAP_COMMON_CFG	1
> >> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c 
> >> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >> index e7fd65f..5b81676 100644
> >> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >> @@ -234,7 +234,7 @@ int virtio_user_stop_device(struct virtio_user_dev
> *dev)
> >>  	uint32_t i, q;
> >>  
> >>  	dev->vhostfd = -1;
> >> -	for (i = 0; i < VIRTIO_MAX_VIRTQUEUES * 2 + 1; ++i) {
> >> +	for (i = 0; i < VIRTIO_MAX_VIRTQUEUES; ++i) {
> >
> >Right, we don't need setup callfd and kickfd for the ctrl-queue.
> 
> I did not remove the ctrl-queue. I just redefined the MACRO according to DRY
> principle:

Oh, right, sorry, I overlooked it. Then for this patch, it's okay to me.

> #define VIRTIO_MAX_VIRTQUEUES VIRTIO_MAX_VIRTQUEUE_PAIRS * 2 + 1
> I noticed that I missed the bracket in the MACRO and will add it in next
> version.

Yes, please. Also, please put a "Cc: stable@dpdk.org" before your
Signed-of-by: it's a candidate for stable releases.

	--yliu