[PATCH] net/virtio: fix crash when dev is configured twice

Alexander Chernavin

author

2022-08-31 08:54 UTC #0 lore raw

PATCH 1/1 net/virtio: fix crash when dev is configured twice

When first attempt to configure a device with RX interrupt enabled fails for some reason (e.g. because "Multiple intr vector not supported"), second attempt to configure the device with RX interrupt disabled and feature set unchanged will succeed but will leave virtio queues not allocated. Accessing the queues will cause a segfault.

First attempt:
- virtio_dev_configure()
- virtio_init_device() is called to reinit the device because "dev->data->dev_conf.intr_conf.rxq" is "1"
- virtio_configure_intr() fails and returns an error
- virtio_free_queues() frees previously allocated virtio queues
- virtio_init_device() fails and returns an error
- virtio_dev_configure() fails and returns an error

Second attempt:
- virtio_dev_configure()
- This time virtio_init_device() is not called, virtio queues are not allocated

With this fix, reinit the device during configuration if virtio queues are not allocated.

Signed-off-by Alexander Chernavin <[email protected]>

---
 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/net/virtio/virtio_ethdev.c +7 -0

index d180162abd..38bfe050b5 100644
@@ -2616,6 +2616,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 			return ret;
 	}
 
+	/* if queues are not allocated, reinit the device */
+	if (hw->vqs == NULL) {
+		ret = virtio_init_device(dev, hw->req_guest_features);
+		if (ret < 0)
+			return ret;
+	}
+
 	if ((rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) &&
 			!virtio_with_feature(hw, VIRTIO_NET_F_RSS)) {
 		PMD_DRV_LOG(ERR, "RSS support requested but not supported by the device");

Signature · 1 line

2.25.1

Xia, Chenbo

2022-09-26 07:15 UTC #1 lore raw

Quoted 22 lines

-----Original Message-----
From: Alexander Chernavin <[email protected]>
Sent: Wednesday, August 31, 2022 4:54 PM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Alexander Chernavin <[email protected]>
Subject: [PATCH] net/virtio: fix crash when dev is configured twice
When first attempt to configure a device with RX interrupt enabled fails for some reason (e.g. because "Multiple intr vector not supported"), second attempt to configure the device with RX interrupt disabled and feature set unchanged will succeed but will leave virtio queues not allocated. Accessing the queues will cause a segfault.
First attempt:
- virtio_dev_configure()
- virtio_init_device() is called to reinit the device because "dev->data->dev_conf.intr_conf.rxq" is "1"
- virtio_configure_intr() fails and returns an error
- virtio_free_queues() frees previously allocated virtio queues
- virtio_init_device() fails and returns an error
- virtio_dev_configure() fails and returns an error

Quoted 7 lines

Second attempt:
- virtio_dev_configure()
- This time virtio_init_device() is not called, virtio queues are not allocated
With this fix, reinit the device during configuration if virtio queues are not allocated.

Good catch. This is a fix, we should backport it, let's add fix and cc-stable Tag.

Thanks,
Chenbo

Quoted 28 lines

Signed-off-by Alexander Chernavin <[email protected]>

---
 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/net/virtio/virtio_ethdev.c +7 -0

index d180162abd..38bfe050b5 100644
@@ -2616,6 +2616,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 			return ret;
 	}
 
+	/* if queues are not allocated, reinit the device */
+	if (hw->vqs == NULL) {
+		ret = virtio_init_device(dev, hw->req_guest_features);
+		if (ret < 0)
+			return ret;
+	}
+
 	if ((rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) &&
 			!virtio_with_feature(hw, VIRTIO_NET_F_RSS)) {
 		PMD_DRV_LOG(ERR, "RSS support requested but not supported by

the device");
--
2.25.1

Alexander Chernavin

author

2022-09-26 08:33 UTC #2 lore raw

PATCH v2 1/1 net/virtio: fix crash when dev is configured twice

In reply to Alexander Chernavin

When first attempt to configure a device with RX interrupt enabled fails for some reason (e.g. because "Multiple intr vector not supported"), second attempt to configure the device with RX interrupt disabled and feature set unchanged will succeed but will leave virtio queues not allocated. Accessing the queues will cause a segfault.

First attempt:
- virtio_dev_configure()
- virtio_init_device() is called to reinit the device because "dev->data->dev_conf.intr_conf.rxq" is "1"
- virtio_configure_intr() fails and returns an error
- virtio_free_queues() frees previously allocated virtio queues
- virtio_init_device() fails and returns an error
- virtio_dev_configure() fails and returns an error

Second attempt:
- virtio_dev_configure()
- This time virtio_init_device() is not called, virtio queues are not allocated

With this fix, reinit the device during configuration if virtio queues are not allocated.

Cc [email protected]

Signed-off-by Alexander Chernavin <[email protected]>

---
v2:
* Also CC to [email protected]

 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/net/virtio/virtio_ethdev.c +7 -0

index d180162abd..38bfe050b5 100644
@@ -2616,6 +2616,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 			return ret;
 	}
 
+	/* if queues are not allocated, reinit the device */
+	if (hw->vqs == NULL) {
+		ret = virtio_init_device(dev, hw->req_guest_features);
+		if (ret < 0)
+			return ret;
+	}
+
 	if ((rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) &&
 			!virtio_with_feature(hw, VIRTIO_NET_F_RSS)) {
 		PMD_DRV_LOG(ERR, "RSS support requested but not supported by the device");

Signature · 1 line

2.25.1

Alexander Chernavin

author

2022-09-27 10:15 UTC #3 lore raw

PATCH v3 1/1 net/virtio: fix crash when dev is configured twice

When first attempt to configure a device with RX interrupt enabled fails for some reason (e.g. because "Multiple intr vector not supported"), second attempt to configure the device with RX interrupt disabled and feature set unchanged will succeed but will leave virtio queues not allocated. Accessing the queues will cause a segfault.

First attempt:
- virtio_dev_configure()
- virtio_init_device() is called to reinit the device because "dev->data->dev_conf.intr_conf.rxq" is "1"
- virtio_configure_intr() fails and returns an error
- virtio_free_queues() frees previously allocated virtio queues
- virtio_init_device() fails and returns an error
- virtio_dev_configure() fails and returns an error

Second attempt:
- virtio_dev_configure()
- This time virtio_init_device() is not called, virtio queues are not allocated

With this fix, reinit the device during configuration if virtio queues are not allocated.

Fixes 2b38151f745a ("net/virtio: fix queue memory leak on error")

Cc [email protected]

Signed-off-by Alexander Chernavin <[email protected]>

---
v2: Add Cc: [email protected]
v3: Add Fixes: 2b38151f745a

 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/net/virtio/virtio_ethdev.c +7 -0

index d180162abd..38bfe050b5 100644
@@ -2616,6 +2616,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 			return ret;
 	}
 
+	/* if queues are not allocated, reinit the device */
+	if (hw->vqs == NULL) {
+		ret = virtio_init_device(dev, hw->req_guest_features);
+		if (ret < 0)
+			return ret;
+	}
+
 	if ((rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) &&
 			!virtio_with_feature(hw, VIRTIO_NET_F_RSS)) {
 		PMD_DRV_LOG(ERR, "RSS support requested but not supported by the device");

Signature · 1 line

2.25.1

Xia, Chenbo

Reviewed-by on v3 1/1

2022-09-28 04:47 UTC #4 lore raw

Quoted 61 lines

-----Original Message-----
From: Alexander Chernavin <[email protected]>
Sent: Tuesday, September 27, 2022 6:15 PM
To: Xia, Chenbo <[email protected]>; [email protected]
Cc: [email protected]; Alexander Chernavin <[email protected]>; [email protected]
Subject: [PATCH v3] net/virtio: fix crash when dev is configured twice
When first attempt to configure a device with RX interrupt enabled fails for some reason (e.g. because "Multiple intr vector not supported"), second attempt to configure the device with RX interrupt disabled and feature set unchanged will succeed but will leave virtio queues not allocated. Accessing the queues will cause a segfault.
First attempt:
- virtio_dev_configure()
- virtio_init_device() is called to reinit the device because "dev->data->dev_conf.intr_conf.rxq" is "1"
- virtio_configure_intr() fails and returns an error
- virtio_free_queues() frees previously allocated virtio queues
- virtio_init_device() fails and returns an error
- virtio_dev_configure() fails and returns an error
Second attempt:
- virtio_dev_configure()
- This time virtio_init_device() is not called, virtio queues are not allocated
With this fix, reinit the device during configuration if virtio queues are not allocated.
Fixes 2b38151f745a ("net/virtio: fix queue memory leak on error")
Cc [email protected]
Signed-off-by Alexander Chernavin <[email protected]>
---
v2: Add Cc: [email protected]
v3: Add Fixes: 2b38151f745a
 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/net/virtio/virtio_ethdev.c +7 -0
index d180162abd..38bfe050b5 100644
@@ -2616,6 +2616,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 			return ret;
 	}
 
+	/* if queues are not allocated, reinit the device */
+	if (hw->vqs == NULL) {
+		ret = virtio_init_device(dev, hw->req_guest_features);
+		if (ret < 0)
+			return ret;
+	}
+
 	if ((rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) &&
 			!virtio_with_feature(hw, VIRTIO_NET_F_RSS)) {
 		PMD_DRV_LOG(ERR, "RSS support requested but not supported by

Quoted 3 lines

the device");
--
2.25.1

Reviewed-by Chenbo Xia <[email protected]>

Xia, Chenbo

Applied

2022-09-29 08:33 UTC #5 lore raw

Quoted 59 lines

-----Original Message-----
From: Xia, Chenbo <[email protected]>
Sent: Wednesday, September 28, 2022 12:47 PM
To: Alexander Chernavin <[email protected]>; [email protected]
Cc: [email protected]; [email protected]
Subject: RE: [PATCH v3] net/virtio: fix crash when dev is configured twice
Quoted 51 lines
-----Original Message-----
From: Alexander Chernavin <[email protected]>
Sent: Tuesday, September 27, 2022 6:15 PM
To: Xia, Chenbo <[email protected]>; [email protected]
Cc: [email protected]; Alexander Chernavin <[email protected]>; [email protected]
Subject: [PATCH v3] net/virtio: fix crash when dev is configured twice
When first attempt to configure a device with RX interrupt enabled fails for some reason (e.g. because "Multiple intr vector not supported"), second attempt to configure the device with RX interrupt disabled and feature set unchanged will succeed but will leave virtio queues not allocated. Accessing the queues will cause a segfault.
First attempt:
- virtio_dev_configure()
- virtio_init_device() is called to reinit the device because "dev->data->dev_conf.intr_conf.rxq" is "1"
- virtio_configure_intr() fails and returns an error
- virtio_free_queues() frees previously allocated virtio queues
- virtio_init_device() fails and returns an error
- virtio_dev_configure() fails and returns an error
Second attempt:
- virtio_dev_configure()
- This time virtio_init_device() is not called, virtio queues are not allocated
With this fix, reinit the device during configuration if virtio queues are not allocated.
Fixes 2b38151f745a ("net/virtio: fix queue memory leak on error")
Cc [email protected]
Signed-off-by Alexander Chernavin <[email protected]>
---
v2: Add Cc: [email protected]
v3: Add Fixes: 2b38151f745a
 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/net/virtio/virtio_ethdev.c +0 -0
index d180162abd..38bfe050b5 100644
@@ -2616,6 +2616,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 			return ret;
 	}
 

Quoted 15 lines

Quoted 10 lines

drivers/net/virtio/virtio_ethdev.c +7 -0

+	/* if queues are not allocated, reinit the device */
+	if (hw->vqs == NULL) {
+		ret = virtio_init_device(dev, hw->req_guest_features);
+		if (ret < 0)
+			return ret;
+	}
+
 	if ((rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) &&
 			!virtio_with_feature(hw, VIRTIO_NET_F_RSS)) {
 		PMD_DRV_LOG(ERR, "RSS support requested but not supported by

Quoted 3 lines

the device");
--
2.25.1

Reviewed-by Chenbo Xia <[email protected]>

Applied to next-virtio/main, thanks