public inbox for dev@dpdk.org
 help / color / mirror / Atom feed
* [PATCH] net/tap: fix close for partially configured port
@ 2026-03-26 12:46 David Marchand
  2026-03-26 13:40 ` Bruce Richardson
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: David Marchand @ 2026-03-26 12:46 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

In case no rxq has been set up (like when starting testpmd with no mempool
drivers), a crash happens in tap_dev_close:

Thread 1 "dpdk-testpmd" received signal SIGSEGV, Segmentation fault.
0x00007ffff7fad68b in tap_dev_close (dev=dev@entry=0x4c4a80
	<rte_eth_devices@INTERNAL>) at ../drivers/net/tap/rte_eth_tap.c:1111
1111			struct rx_queue *rxq = dev->data->rx_queues[i];

(gdb) p dev->data->rx_queues
$4 = (void **) 0x0

Fixes: 23e2387b49a1 ("net/tap: allocate queue structures dynamically")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/tap/rte_eth_tap.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 64b359914b..6d710131e8 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1108,17 +1108,22 @@ tap_dev_close(struct rte_eth_dev *dev)
 #endif
 
 	for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
-		struct rx_queue *rxq = dev->data->rx_queues[i];
+		struct rx_queue *rxq = NULL;
+		struct rx_queue *txq = NULL;
 
 		tap_queue_close(process_private, i);
 
+		if (dev->data->rx_queues != NULL)
+			rxq = dev->data->rx_queues[i];
 		if (rxq != NULL) {
 			tap_rxq_pool_free(rxq->pool);
 			rte_free(rxq);
 			dev->data->rx_queues[i] = NULL;
 		}
 
-		if (dev->data->tx_queues[i] != NULL) {
+		if (dev->data->tx_queues != NULL)
+			txq = dev->data->tx_queues[i];
+		if (txq != NULL) {
 			rte_free(dev->data->tx_queues[i]);
 			dev->data->tx_queues[i] = NULL;
 		}
-- 
2.53.0


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

* Re: [PATCH] net/tap: fix close for partially configured port
  2026-03-26 12:46 [PATCH] net/tap: fix close for partially configured port David Marchand
@ 2026-03-26 13:40 ` Bruce Richardson
  2026-03-26 18:31 ` Stephen Hemminger
  2026-03-27  8:39 ` [PATCH v2] " David Marchand
  2 siblings, 0 replies; 7+ messages in thread
From: Bruce Richardson @ 2026-03-26 13:40 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Stephen Hemminger

On Thu, Mar 26, 2026 at 01:46:11PM +0100, David Marchand wrote:
> In case no rxq has been set up (like when starting testpmd with no mempool
> drivers), a crash happens in tap_dev_close:
> 
> Thread 1 "dpdk-testpmd" received signal SIGSEGV, Segmentation fault.
> 0x00007ffff7fad68b in tap_dev_close (dev=dev@entry=0x4c4a80
> 	<rte_eth_devices@INTERNAL>) at ../drivers/net/tap/rte_eth_tap.c:1111
> 1111			struct rx_queue *rxq = dev->data->rx_queues[i];
> 
> (gdb) p dev->data->rx_queues
> $4 = (void **) 0x0
> 
> Fixes: 23e2387b49a1 ("net/tap: allocate queue structures dynamically")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Looks ok to me.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

>  drivers/net/tap/rte_eth_tap.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 64b359914b..6d710131e8 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -1108,17 +1108,22 @@ tap_dev_close(struct rte_eth_dev *dev)
>  #endif
>  
>  	for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
> -		struct rx_queue *rxq = dev->data->rx_queues[i];
> +		struct rx_queue *rxq = NULL;
> +		struct rx_queue *txq = NULL;
>  
>  		tap_queue_close(process_private, i);
>  
> +		if (dev->data->rx_queues != NULL)
> +			rxq = dev->data->rx_queues[i];
>  		if (rxq != NULL) {
>  			tap_rxq_pool_free(rxq->pool);
>  			rte_free(rxq);
>  			dev->data->rx_queues[i] = NULL;
>  		}
>  
> -		if (dev->data->tx_queues[i] != NULL) {
> +		if (dev->data->tx_queues != NULL)
> +			txq = dev->data->tx_queues[i];
> +		if (txq != NULL) {
>  			rte_free(dev->data->tx_queues[i]);
>  			dev->data->tx_queues[i] = NULL;
>  		}
> -- 
> 2.53.0
> 

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

* Re: [PATCH] net/tap: fix close for partially configured port
  2026-03-26 12:46 [PATCH] net/tap: fix close for partially configured port David Marchand
  2026-03-26 13:40 ` Bruce Richardson
@ 2026-03-26 18:31 ` Stephen Hemminger
  2026-03-27  8:29   ` David Marchand
  2026-03-27  8:39 ` [PATCH v2] " David Marchand
  2 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2026-03-26 18:31 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

On Thu, 26 Mar 2026 13:46:11 +0100
David Marchand <david.marchand@redhat.com> wrote:

> In case no rxq has been set up (like when starting testpmd with no mempool
> drivers), a crash happens in tap_dev_close:
> 
> Thread 1 "dpdk-testpmd" received signal SIGSEGV, Segmentation fault.
> 0x00007ffff7fad68b in tap_dev_close (dev=dev@entry=0x4c4a80
> 	<rte_eth_devices@INTERNAL>) at ../drivers/net/tap/rte_eth_tap.c:1111
> 1111			struct rx_queue *rxq = dev->data->rx_queues[i];
> 
> (gdb) p dev->data->rx_queues
> $4 = (void **) 0x0
> 
> Fixes: 23e2387b49a1 ("net/tap: allocate queue structures dynamically")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

It looked ok, but then the AI review spotted some stuff...


Two issues:

The txq variable is declared as struct rx_queue * but should be
struct tx_queue *. Works by accident since it comes from a void *
array and is only NULL-tested and passed to rte_free(), but the
type is wrong.

Pre-existing: the loop runs to RTE_PMD_TAP_MAX_QUEUES but the
rx_queues/tx_queues arrays are allocated with nb_rx_queues /
nb_tx_queues entries by ethdev. If dev_configure() was called
with fewer queues, the arrays are non-NULL but the access is
out-of-bounds. Since these lines are being reworked anyway, worth
adding a bounds check against nb_rx_queues/nb_tx_queues. The
tap_queue_close() call is fine -- process_private fds are sized
to RTE_PMD_TAP_MAX_QUEUES.

Also missing Cc: stable@dpdk.org for a crash fix.

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

* Re: [PATCH] net/tap: fix close for partially configured port
  2026-03-26 18:31 ` Stephen Hemminger
@ 2026-03-27  8:29   ` David Marchand
  0 siblings, 0 replies; 7+ messages in thread
From: David Marchand @ 2026-03-27  8:29 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Bruce Richardson

On Thu, 26 Mar 2026 at 19:32, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Thu, 26 Mar 2026 13:46:11 +0100
> David Marchand <david.marchand@redhat.com> wrote:
>
> > In case no rxq has been set up (like when starting testpmd with no mempool
> > drivers), a crash happens in tap_dev_close:
> >
> > Thread 1 "dpdk-testpmd" received signal SIGSEGV, Segmentation fault.
> > 0x00007ffff7fad68b in tap_dev_close (dev=dev@entry=0x4c4a80
> >       <rte_eth_devices@INTERNAL>) at ../drivers/net/tap/rte_eth_tap.c:1111
> > 1111                  struct rx_queue *rxq = dev->data->rx_queues[i];
> >
> > (gdb) p dev->data->rx_queues
> > $4 = (void **) 0x0
> >
> > Fixes: 23e2387b49a1 ("net/tap: allocate queue structures dynamically")
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
>
> It looked ok, but then the AI review spotted some stuff...
>
>
> Two issues:
>
> The txq variable is declared as struct rx_queue * but should be
> struct tx_queue *. Works by accident since it comes from a void *
> array and is only NULL-tested and passed to rte_free(), but the
> type is wrong.

Ah yes, will send a v2 quickly.

>
> Pre-existing: the loop runs to RTE_PMD_TAP_MAX_QUEUES but the
> rx_queues/tx_queues arrays are allocated with nb_rx_queues /
> nb_tx_queues entries by ethdev. If dev_configure() was called
> with fewer queues, the arrays are non-NULL but the access is
> out-of-bounds. Since these lines are being reworked anyway, worth
> adding a bounds check against nb_rx_queues/nb_tx_queues. The
> tap_queue_close() call is fine -- process_private fds are sized
> to RTE_PMD_TAP_MAX_QUEUES.

Indeed, and that makes the fix even simpler.


>
> Also missing Cc: stable@dpdk.org for a crash fix.

No, this is a fix for a 26.03 regression.


-- 
David Marchand


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

* [PATCH v2] net/tap: fix close for partially configured port
  2026-03-26 12:46 [PATCH] net/tap: fix close for partially configured port David Marchand
  2026-03-26 13:40 ` Bruce Richardson
  2026-03-26 18:31 ` Stephen Hemminger
@ 2026-03-27  8:39 ` David Marchand
  2026-03-27 15:37   ` Stephen Hemminger
  2026-03-27 16:22   ` Stephen Hemminger
  2 siblings, 2 replies; 7+ messages in thread
From: David Marchand @ 2026-03-27  8:39 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

In case no rxq has been set up (like when starting testpmd with no mempool
drivers), a crash happens in tap_dev_close:

Thread 1 "dpdk-testpmd" received signal SIGSEGV, Segmentation fault.
0x00007ffff7fad68b in tap_dev_close (dev=dev@entry=0x4c4a80
	<rte_eth_devices@INTERNAL>) at ../drivers/net/tap/rte_eth_tap.c:1111
1111			struct rx_queue *rxq = dev->data->rx_queues[i];

(gdb) p dev->data->rx_queues
$4 = (void **) 0x0

Fixes: 23e2387b49a1 ("net/tap: allocate queue structures dynamically")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v1:
- as Stephen AI reported, [rt]x_queues array are sized against
  dev->data->nb_[rt]x_queues, so the loop after the 23e2387b49a1 rework
  can go out of bound. Since nb_rx_queues == nb_tx_queues with this
  driver, simply check the number of configured rxq,

---
 drivers/net/tap/rte_eth_tap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 64b359914b..a5d460a0b3 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1107,7 +1107,7 @@ tap_dev_close(struct rte_eth_dev *dev)
 	}
 #endif
 
-	for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		struct rx_queue *rxq = dev->data->rx_queues[i];
 
 		tap_queue_close(process_private, i);
-- 
2.53.0


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

* Re: [PATCH v2] net/tap: fix close for partially configured port
  2026-03-27  8:39 ` [PATCH v2] " David Marchand
@ 2026-03-27 15:37   ` Stephen Hemminger
  2026-03-27 16:22   ` Stephen Hemminger
  1 sibling, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2026-03-27 15:37 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

On Fri, 27 Mar 2026 09:39:02 +0100
David Marchand <david.marchand@redhat.com> wrote:

> In case no rxq has been set up (like when starting testpmd with no mempool
> drivers), a crash happens in tap_dev_close:
> 
> Thread 1 "dpdk-testpmd" received signal SIGSEGV, Segmentation fault.
> 0x00007ffff7fad68b in tap_dev_close (dev=dev@entry=0x4c4a80
> 	<rte_eth_devices@INTERNAL>) at ../drivers/net/tap/rte_eth_tap.c:1111
> 1111			struct rx_queue *rxq = dev->data->rx_queues[i];
> 
> (gdb) p dev->data->rx_queues
> $4 = (void **) 0x0
> 
> Fixes: 23e2387b49a1 ("net/tap: allocate queue structures dynamically")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

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

* Re: [PATCH v2] net/tap: fix close for partially configured port
  2026-03-27  8:39 ` [PATCH v2] " David Marchand
  2026-03-27 15:37   ` Stephen Hemminger
@ 2026-03-27 16:22   ` Stephen Hemminger
  1 sibling, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2026-03-27 16:22 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

On Fri, 27 Mar 2026 09:39:02 +0100
David Marchand <david.marchand@redhat.com> wrote:

> In case no rxq has been set up (like when starting testpmd with no mempool
> drivers), a crash happens in tap_dev_close:
> 
> Thread 1 "dpdk-testpmd" received signal SIGSEGV, Segmentation fault.
> 0x00007ffff7fad68b in tap_dev_close (dev=dev@entry=0x4c4a80
> 	<rte_eth_devices@INTERNAL>) at ../drivers/net/tap/rte_eth_tap.c:1111
> 1111			struct rx_queue *rxq = dev->data->rx_queues[i];
> 
> (gdb) p dev->data->rx_queues
> $4 = (void **) 0x0
> 
> Fixes: 23e2387b49a1 ("net/tap: allocate queue structures dynamically")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v1:
> - as Stephen AI reported, [rt]x_queues array are sized against
>   dev->data->nb_[rt]x_queues, so the loop after the 23e2387b49a1 rework
>   can go out of bound. Since nb_rx_queues == nb_tx_queues with this
>   driver, simply check the number of configured rxq,
> 
> ---

Applied to next-net

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

end of thread, other threads:[~2026-03-27 16:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-26 12:46 [PATCH] net/tap: fix close for partially configured port David Marchand
2026-03-26 13:40 ` Bruce Richardson
2026-03-26 18:31 ` Stephen Hemminger
2026-03-27  8:29   ` David Marchand
2026-03-27  8:39 ` [PATCH v2] " David Marchand
2026-03-27 15:37   ` Stephen Hemminger
2026-03-27 16:22   ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox