DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/virtio: fix crash when dev is configured twice
@ 2022-08-31  8:53 Alexander Chernavin
  2022-09-26  7:15 ` Xia, Chenbo
  2022-09-26  8:32 ` [PATCH v2] " Alexander Chernavin
  0 siblings, 2 replies; 6+ messages in thread
From: Alexander Chernavin @ 2022-08-31  8:53 UTC (permalink / raw)
  To: maxime.coquelin, chenbo.xia; +Cc: dev, 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.

Signed-off-by: Alexander Chernavin <achernavin@netgate.com>
---
 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d180162abd..38bfe050b5 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -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


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

* RE: [PATCH] net/virtio: fix crash when dev is configured twice
  2022-08-31  8:53 [PATCH] net/virtio: fix crash when dev is configured twice Alexander Chernavin
@ 2022-09-26  7:15 ` Xia, Chenbo
  2022-09-26  8:32 ` [PATCH v2] " Alexander Chernavin
  1 sibling, 0 replies; 6+ messages in thread
From: Xia, Chenbo @ 2022-09-26  7:15 UTC (permalink / raw)
  To: Alexander Chernavin, maxime.coquelin; +Cc: dev

> -----Original Message-----
> From: Alexander Chernavin <achernavin@netgate.com>
> Sent: Wednesday, August 31, 2022 4:54 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Alexander Chernavin <achernavin@netgate.com>
> 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
> 
> 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 

> 
> Signed-off-by: Alexander Chernavin <achernavin@netgate.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index d180162abd..38bfe050b5 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -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


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

* [PATCH v2] net/virtio: fix crash when dev is configured twice
  2022-08-31  8:53 [PATCH] net/virtio: fix crash when dev is configured twice Alexander Chernavin
  2022-09-26  7:15 ` Xia, Chenbo
@ 2022-09-26  8:32 ` Alexander Chernavin
  2022-09-27 10:15   ` [PATCH v3] " Alexander Chernavin
  1 sibling, 1 reply; 6+ messages in thread
From: Alexander Chernavin @ 2022-09-26  8:32 UTC (permalink / raw)
  To: maxime.coquelin, chenbo.xia; +Cc: dev, Alexander Chernavin, stable

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: stable@dpdk.org

Signed-off-by: Alexander Chernavin <achernavin@netgate.com>
---
v2:
* Also CC to stable@dpdk.org

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

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d180162abd..38bfe050b5 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -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


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

* [PATCH v3] net/virtio: fix crash when dev is configured twice
  2022-09-26  8:32 ` [PATCH v2] " Alexander Chernavin
@ 2022-09-27 10:15   ` Alexander Chernavin
  2022-09-28  4:47     ` Xia, Chenbo
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander Chernavin @ 2022-09-27 10:15 UTC (permalink / raw)
  To: chenbo.xia, maxime.coquelin; +Cc: dev, Alexander Chernavin, stable

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: stable@dpdk.org

Signed-off-by: Alexander Chernavin <achernavin@netgate.com>
---
v2: Add Cc: stable@dpdk.org
v3: Add Fixes: 2b38151f745a

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

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d180162abd..38bfe050b5 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -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


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

* RE: [PATCH v3] net/virtio: fix crash when dev is configured twice
  2022-09-27 10:15   ` [PATCH v3] " Alexander Chernavin
@ 2022-09-28  4:47     ` Xia, Chenbo
  2022-09-29  8:33       ` Xia, Chenbo
  0 siblings, 1 reply; 6+ messages in thread
From: Xia, Chenbo @ 2022-09-28  4:47 UTC (permalink / raw)
  To: Alexander Chernavin, maxime.coquelin; +Cc: dev, stable

> -----Original Message-----
> From: Alexander Chernavin <achernavin@netgate.com>
> Sent: Tuesday, September 27, 2022 6:15 PM
> To: Xia, Chenbo <chenbo.xia@intel.com>; maxime.coquelin@redhat.com
> Cc: dev@dpdk.org; Alexander Chernavin <achernavin@netgate.com>;
> stable@dpdk.org
> 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: stable@dpdk.org
> 
> Signed-off-by: Alexander Chernavin <achernavin@netgate.com>
> ---
> v2: Add Cc: stable@dpdk.org
> v3: Add Fixes: 2b38151f745a
> 
>  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index d180162abd..38bfe050b5 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -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

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

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

* RE: [PATCH v3] net/virtio: fix crash when dev is configured twice
  2022-09-28  4:47     ` Xia, Chenbo
@ 2022-09-29  8:33       ` Xia, Chenbo
  0 siblings, 0 replies; 6+ messages in thread
From: Xia, Chenbo @ 2022-09-29  8:33 UTC (permalink / raw)
  To: Alexander Chernavin, maxime.coquelin; +Cc: dev, stable

> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Wednesday, September 28, 2022 12:47 PM
> To: Alexander Chernavin <achernavin@netgate.com>;
> maxime.coquelin@redhat.com
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH v3] net/virtio: fix crash when dev is configured twice
> 
> > -----Original Message-----
> > From: Alexander Chernavin <achernavin@netgate.com>
> > Sent: Tuesday, September 27, 2022 6:15 PM
> > To: Xia, Chenbo <chenbo.xia@intel.com>; maxime.coquelin@redhat.com
> > Cc: dev@dpdk.org; Alexander Chernavin <achernavin@netgate.com>;
> > stable@dpdk.org
> > 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: stable@dpdk.org
> >
> > Signed-off-by: Alexander Chernavin <achernavin@netgate.com>
> > ---
> > v2: Add Cc: stable@dpdk.org
> > v3: Add Fixes: 2b38151f745a
> >
> >  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c
> > b/drivers/net/virtio/virtio_ethdev.c
> > index d180162abd..38bfe050b5 100644
> > --- a/drivers/net/virtio/virtio_ethdev.c
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> > @@ -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
> 
> Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

Applied to next-virtio/main, thanks

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

end of thread, other threads:[~2022-09-29  8:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-31  8:53 [PATCH] net/virtio: fix crash when dev is configured twice Alexander Chernavin
2022-09-26  7:15 ` Xia, Chenbo
2022-09-26  8:32 ` [PATCH v2] " Alexander Chernavin
2022-09-27 10:15   ` [PATCH v3] " Alexander Chernavin
2022-09-28  4:47     ` Xia, Chenbo
2022-09-29  8:33       ` Xia, Chenbo

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).