diff mbox series

[v2] eventdev/rx-adapter: segfault in queue conf get

Message ID 20210930130001.430924-1-ganapati.kundapura@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers
Series [v2] eventdev/rx-adapter: segfault in queue conf get |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ganapati Kundapura Sept. 30, 2021, 1 p.m. UTC 
  rte_event_eth_rx_adapter_queue_conf_get() segfaults if called
without queue added to the Rx adapter.

Added check to no queues in Rx adapter and error out on being
called with no queue in Rx adapter.

Added test case to call queue conf get without queues in
Rx adapter.

Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>

---
v2:
* Corrected typo in the comment
---

Comments

Naga Harish K, S V Oct. 1, 2021, 4:42 a.m. UTC | #1 
Hi Ganapati,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ganapati Kundapura
> Sent: Thursday, September 30, 2021 6:30 PM
> To: jerinjacobk@gmail.com; dev@dpdk.org
> Cc: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Subject: [dpdk-dev] [PATCH v2] eventdev/rx-adapter: segfault in queue conf
> get

The commit message can be "fix segfault in queue conf get API"

> 
> rte_event_eth_rx_adapter_queue_conf_get() segfaults if called without
> queue added to the Rx adapter.
> 
> Added check to no queues in Rx adapter and error out on being called with
> no queue in Rx adapter.
> 
> Added test case to call queue conf get without queues in Rx adapter.
> 

You may need to mention the commit details which is getting fixed.

> Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> 
> ---
> v2:
> * Corrected typo in the comment
> ---
> 
> diff --git a/app/test/test_event_eth_rx_adapter.c
> b/app/test/test_event_eth_rx_adapter.c
> index 13664a3..d0dc552 100644
> --- a/app/test/test_event_eth_rx_adapter.c
> +++ b/app/test/test_event_eth_rx_adapter.c
> @@ -751,20 +751,48 @@ static int
>  adapter_queue_conf(void)
>  {
>  	int err;
> -	struct rte_event_eth_rx_adapter_queue_conf queue_conf;
> +	struct rte_event_eth_rx_adapter_queue_conf queue_conf = {0};
> 
> -	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> TEST_DEV_ID,
> +	/* Case 1: queue conf get without any queues in Rx adapter */
> +	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> +						      TEST_ETHDEV_ID,
> +						      0, &queue_conf);
> +	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
> +
> +	/* Add queue to Rx adapter */
> +	queue_conf.ev.queue_id = 0;
> +	queue_conf.ev.sched_type = RTE_SCHED_TYPE_ATOMIC;
> +	queue_conf.ev.priority = RTE_EVENT_DEV_PRIORITY_NORMAL;
> +
> +	err = rte_event_eth_rx_adapter_queue_add(TEST_INST_ID,
> +						 TEST_ETHDEV_ID,
> +						 0, &queue_conf);
> +	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> +
> +	/* Case 2: queue conf get with queue added to Rx adapter */
> +	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> +						      TEST_ETHDEV_ID,
>  						      0, &queue_conf);
>  	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> 
> -	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> TEST_DEV_ID,
> +	/* Case 3: queue conf get with invalid rx queue id */
> +	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> +						      TEST_ETHDEV_ID,
>  						      -1, &queue_conf);
>  	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
> 
> -	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> TEST_DEV_ID,
> +	/* Case 4: queue conf get with NULL queue conf struct */
> +	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> +						      TEST_ETHDEV_ID,
>  						      0, NULL);
>  	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
> 
> +	/* Delete queue from the Rx adapter */
> +	err = rte_event_eth_rx_adapter_queue_del(TEST_INST_ID,
> +						 TEST_ETHDEV_ID,
> +						 0);
> +	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> +
>  	return TEST_SUCCESS;
>  }
> 
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> b/lib/eventdev/rte_event_eth_rx_adapter.c
> index 10491ca..2a84490 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -2844,12 +2844,13 @@
> rte_event_eth_rx_adapter_queue_conf_get(uint8_t id,
>  		return -EINVAL;
> 
>  	dev_info = &rx_adapter->eth_devices[eth_dev_id];
> -	queue_info = &dev_info->rx_queue[rx_queue_id];
> -	if (!queue_info->queue_enabled) {
> +	if (dev_info->rx_queue == NULL ||
> +	    !dev_info->rx_queue[rx_queue_id].queue_enabled) {
>  		RTE_EDEV_LOG_ERR("Rx queue %u not added",
> rx_queue_id);
>  		return -EINVAL;
>  	}
> 
> +	queue_info = &dev_info->rx_queue[rx_queue_id];
>  	qi_ev = (struct rte_event *)&queue_info->event;
> 
>  	memset(queue_conf, 0, sizeof(*queue_conf));
> --
> 2.6.4
Ganapati Kundapura Oct. 1, 2021, 5:21 a.m. UTC | #2 
Hi Harish,

> -----Original Message-----
> From: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Sent: 01 October 2021 10:12
> To: Kundapura, Ganapati <ganapati.kundapura@intel.com>;
> jerinjacobk@gmail.com; dev@dpdk.org
> Cc: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2] eventdev/rx-adapter: segfault in queue
> conf get
> 
> Hi Ganapati,
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Ganapati Kundapura
> > Sent: Thursday, September 30, 2021 6:30 PM
> > To: jerinjacobk@gmail.com; dev@dpdk.org
> > Cc: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> > Subject: [dpdk-dev] [PATCH v2] eventdev/rx-adapter: segfault in queue
> > conf get
> 
> The commit message can be "fix segfault in queue conf get API"
> 
> >
> > rte_event_eth_rx_adapter_queue_conf_get() segfaults if called without
> > queue added to the Rx adapter.
> >
> > Added check to no queues in Rx adapter and error out on being called
> > with no queue in Rx adapter.
> >
> > Added test case to call queue conf get without queues in Rx adapter.
> >
> 
> You may need to mention the commit details which is getting fixed.
> 
Done

> > Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> >
> > ---
> > v2:
> > * Corrected typo in the comment
> > ---
> >
> > diff --git a/app/test/test_event_eth_rx_adapter.c
> > b/app/test/test_event_eth_rx_adapter.c
> > index 13664a3..d0dc552 100644
> > --- a/app/test/test_event_eth_rx_adapter.c
> > +++ b/app/test/test_event_eth_rx_adapter.c
> > @@ -751,20 +751,48 @@ static int
> >  adapter_queue_conf(void)
> >  {
> >  	int err;
> > -	struct rte_event_eth_rx_adapter_queue_conf queue_conf;
> > +	struct rte_event_eth_rx_adapter_queue_conf queue_conf = {0};
> >
> > -	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> > TEST_DEV_ID,
> > +	/* Case 1: queue conf get without any queues in Rx adapter */
> > +	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> > +						      TEST_ETHDEV_ID,
> > +						      0, &queue_conf);
> > +	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
> > +
> > +	/* Add queue to Rx adapter */
> > +	queue_conf.ev.queue_id = 0;
> > +	queue_conf.ev.sched_type = RTE_SCHED_TYPE_ATOMIC;
> > +	queue_conf.ev.priority = RTE_EVENT_DEV_PRIORITY_NORMAL;
> > +
> > +	err = rte_event_eth_rx_adapter_queue_add(TEST_INST_ID,
> > +						 TEST_ETHDEV_ID,
> > +						 0, &queue_conf);
> > +	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> > +
> > +	/* Case 2: queue conf get with queue added to Rx adapter */
> > +	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> > +						      TEST_ETHDEV_ID,
> >  						      0, &queue_conf);
> >  	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> >
> > -	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> > TEST_DEV_ID,
> > +	/* Case 3: queue conf get with invalid rx queue id */
> > +	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> > +						      TEST_ETHDEV_ID,
> >  						      -1, &queue_conf);
> >  	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
> >
> > -	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> > TEST_DEV_ID,
> > +	/* Case 4: queue conf get with NULL queue conf struct */
> > +	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
> > +						      TEST_ETHDEV_ID,
> >  						      0, NULL);
> >  	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
> >
> > +	/* Delete queue from the Rx adapter */
> > +	err = rte_event_eth_rx_adapter_queue_del(TEST_INST_ID,
> > +						 TEST_ETHDEV_ID,
> > +						 0);
> > +	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> > +
> >  	return TEST_SUCCESS;
> >  }
> >
> > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > index 10491ca..2a84490 100644
> > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > @@ -2844,12 +2844,13 @@
> > rte_event_eth_rx_adapter_queue_conf_get(uint8_t id,
> >  		return -EINVAL;
> >
> >  	dev_info = &rx_adapter->eth_devices[eth_dev_id];
> > -	queue_info = &dev_info->rx_queue[rx_queue_id];
> > -	if (!queue_info->queue_enabled) {
> > +	if (dev_info->rx_queue == NULL ||
> > +	    !dev_info->rx_queue[rx_queue_id].queue_enabled) {
> >  		RTE_EDEV_LOG_ERR("Rx queue %u not added",
> rx_queue_id);
> >  		return -EINVAL;
> >  	}
> >
> > +	queue_info = &dev_info->rx_queue[rx_queue_id];
> >  	qi_ev = (struct rte_event *)&queue_info->event;
> >
> >  	memset(queue_conf, 0, sizeof(*queue_conf));
> > --
> > 2.6.4
diff mbox series

Patch

diff --git a/app/test/test_event_eth_rx_adapter.c b/app/test/test_event_eth_rx_adapter.c
index 13664a3..d0dc552 100644
--- a/app/test/test_event_eth_rx_adapter.c
+++ b/app/test/test_event_eth_rx_adapter.c
@@ -751,20 +751,48 @@  static int
 adapter_queue_conf(void)
 {
 	int err;
-	struct rte_event_eth_rx_adapter_queue_conf queue_conf;
+	struct rte_event_eth_rx_adapter_queue_conf queue_conf = {0};
 
-	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID, TEST_DEV_ID,
+	/* Case 1: queue conf get without any queues in Rx adapter */
+	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
+						      TEST_ETHDEV_ID,
+						      0, &queue_conf);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
+
+	/* Add queue to Rx adapter */
+	queue_conf.ev.queue_id = 0;
+	queue_conf.ev.sched_type = RTE_SCHED_TYPE_ATOMIC;
+	queue_conf.ev.priority = RTE_EVENT_DEV_PRIORITY_NORMAL;
+
+	err = rte_event_eth_rx_adapter_queue_add(TEST_INST_ID,
+						 TEST_ETHDEV_ID,
+						 0, &queue_conf);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	/* Case 2: queue conf get with queue added to Rx adapter */
+	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
+						      TEST_ETHDEV_ID,
 						      0, &queue_conf);
 	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
 
-	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID, TEST_DEV_ID,
+	/* Case 3: queue conf get with invalid rx queue id */
+	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
+						      TEST_ETHDEV_ID,
 						      -1, &queue_conf);
 	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
 
-	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID, TEST_DEV_ID,
+	/* Case 4: queue conf get with NULL queue conf struct */
+	err = rte_event_eth_rx_adapter_queue_conf_get(TEST_INST_ID,
+						      TEST_ETHDEV_ID,
 						      0, NULL);
 	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
 
+	/* Delete queue from the Rx adapter */
+	err = rte_event_eth_rx_adapter_queue_del(TEST_INST_ID,
+						 TEST_ETHDEV_ID,
+						 0);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
 	return TEST_SUCCESS;
 }
 
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index 10491ca..2a84490 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -2844,12 +2844,13 @@  rte_event_eth_rx_adapter_queue_conf_get(uint8_t id,
 		return -EINVAL;
 
 	dev_info = &rx_adapter->eth_devices[eth_dev_id];
-	queue_info = &dev_info->rx_queue[rx_queue_id];
-	if (!queue_info->queue_enabled) {
+	if (dev_info->rx_queue == NULL ||
+	    !dev_info->rx_queue[rx_queue_id].queue_enabled) {
 		RTE_EDEV_LOG_ERR("Rx queue %u not added", rx_queue_id);
 		return -EINVAL;
 	}
 
+	queue_info = &dev_info->rx_queue[rx_queue_id];
 	qi_ev = (struct rte_event *)&queue_info->event;
 
 	memset(queue_conf, 0, sizeof(*queue_conf));