patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH v2] net/virtio-user: fix overflow
@ 2017-03-14 10:09 Wenfeng Liu
  2017-03-15  8:08 ` Yuanhan Liu
  0 siblings, 1 reply; 4+ messages in thread
From: Wenfeng Liu @ 2017-03-14 10:09 UTC (permalink / raw)
  To: yuanhan.liu, maxime.coquelin; +Cc: dev, stable

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.

Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
Cc: stable@dpdk.org

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..1302556 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) {
 		dev->kickfds[i] = -1;
 		dev->callfds[i] = -1;
 	}
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 6ecb91e..ba80d05 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -49,8 +49,8 @@ struct virtio_user_dev {
 	int		*tapfds;
 
 	/* for both vhost_user and vhost_kernel */
-	int		callfds[VIRTIO_MAX_VIRTQUEUES * 2 + 1];
-	int		kickfds[VIRTIO_MAX_VIRTQUEUES * 2 + 1];
+	int		callfds[VIRTIO_MAX_VIRTQUEUES];
+	int		kickfds[VIRTIO_MAX_VIRTQUEUES];
 	int		mac_specified;
 	uint32_t	max_queue_pairs;
 	uint32_t	queue_pairs;
@@ -62,7 +62,7 @@ struct virtio_user_dev {
 	uint8_t		status;
 	uint8_t		mac_addr[ETHER_ADDR_LEN];
 	char		path[PATH_MAX];
-	struct vring	vrings[VIRTIO_MAX_VIRTQUEUES * 2 + 1];
+	struct vring	vrings[VIRTIO_MAX_VIRTQUEUES];
 	struct virtio_user_backend_ops *ops;
 };
 
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 16d1526..d476a2d 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -433,6 +433,13 @@
 		goto end;
 	}
 
+	if (queues > VIRTIO_MAX_VIRTQUEUE_PAIRS) {
+		PMD_INIT_LOG(ERR, "arg %s %u exceeds the limit %u",
+			VIRTIO_USER_ARG_QUEUES_NUM, queues,
+			VIRTIO_MAX_VIRTQUEUE_PAIRS);
+		goto end;
+	}
+
 	eth_dev = virtio_user_eth_dev_alloc(name);
 	if (!eth_dev) {
 		PMD_INIT_LOG(ERR, "virtio_user fails to alloc device");
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-stable] [PATCH v2] net/virtio-user: fix overflow
  2017-03-14 10:09 [dpdk-stable] [PATCH v2] net/virtio-user: fix overflow Wenfeng Liu
@ 2017-03-15  8:08 ` Yuanhan Liu
  2017-04-05  0:22   ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
  0 siblings, 1 reply; 4+ messages in thread
From: Yuanhan Liu @ 2017-03-15  8:08 UTC (permalink / raw)
  To: Wenfeng Liu; +Cc: maxime.coquelin, dev, stable

On Tue, Mar 14, 2017 at 10:09:56AM +0000, Wenfeng Liu wrote:
> 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.
> 
> Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>

Applied to dpdk-next-virtio.

Thanks.

	--yliu

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] net/virtio-user: fix overflow
  2017-03-15  8:08 ` Yuanhan Liu
@ 2017-04-05  0:22   ` Thomas Monjalon
  2017-04-05  5:23     ` Yuanhan Liu
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Monjalon @ 2017-04-05  0:22 UTC (permalink / raw)
  To: Yuanhan Liu, Wenfeng Liu; +Cc: dev, maxime.coquelin, stable

2017-03-15 16:08, Yuanhan Liu:
> On Tue, Mar 14, 2017 at 10:09:56AM +0000, Wenfeng Liu wrote:
> > 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.
> > 
> > Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>
> 
> Applied to dpdk-next-virtio.

There is a compilation error when RTE_LIBRTE_VIRTIO_DEBUG_INIT is enabled:
drivers/net/virtio/virtio_user_ethdev.c:423:32:
fatal error: format specifies type 'unsigned int'
but the argument has type 'uint64_t' (aka 'unsigned long') [-Wformat]
                        VIRTIO_USER_ARG_QUEUES_NUM, queues,
                                                    ^~~~~~

I will fix it when pulling next-virtio.

Considering this log is about an error,
does it make sense to have a compile option to enable it?
I was expecting that "git grep 'PMD_INIT_LOG(ERR,' | wc -l" would return 0.
Unfortunately, there are 434 occurences...

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] net/virtio-user: fix overflow
  2017-04-05  0:22   ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
@ 2017-04-05  5:23     ` Yuanhan Liu
  0 siblings, 0 replies; 4+ messages in thread
From: Yuanhan Liu @ 2017-04-05  5:23 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Wenfeng Liu, dev, maxime.coquelin, stable

On Wed, Apr 05, 2017 at 02:22:27AM +0200, Thomas Monjalon wrote:
> 2017-03-15 16:08, Yuanhan Liu:
> > On Tue, Mar 14, 2017 at 10:09:56AM +0000, Wenfeng Liu wrote:
> > > 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.
> > > 
> > > Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>
> > 
> > Applied to dpdk-next-virtio.
> 
> There is a compilation error when RTE_LIBRTE_VIRTIO_DEBUG_INIT is enabled:
> drivers/net/virtio/virtio_user_ethdev.c:423:32:
> fatal error: format specifies type 'unsigned int'
> but the argument has type 'uint64_t' (aka 'unsigned long') [-Wformat]
>                         VIRTIO_USER_ARG_QUEUES_NUM, queues,
>                                                     ^~~~~~
> 
> I will fix it when pulling next-virtio.

Thanks!

> Considering this log is about an error,
> does it make sense to have a compile option to enable it?

I think it makes more sense to print them unconditionally? They are
errors after all.

	--yliu

> I was expecting that "git grep 'PMD_INIT_LOG(ERR,' | wc -l" would return 0.
> Unfortunately, there are 434 occurences...

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-04-05  5:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-14 10:09 [dpdk-stable] [PATCH v2] net/virtio-user: fix overflow Wenfeng Liu
2017-03-15  8:08 ` Yuanhan Liu
2017-04-05  0:22   ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
2017-04-05  5:23     ` Yuanhan Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).