[PATCH RESEND v2 00/11] codeql fixes for various subsystems

[PATCH RESEND v2 00/11] codeql fixes for various subsystems

[okaya]

From: Sinan Kaya <okaya@kernel.org>

Following up the codeql reported problems first submitted
by Stephen Hemminger here:

https://lore.kernel.org/all/20220527161210.77212d0b@hermes.local/t/

Posting a series of fixes about potential null pointer accesses.

Changes from v1:
- Remove braces around single line statements
- use NULL comparisons

Sinan Kaya (11):
  ethdev: check return result of rte_eth_dev_info_get
  net/tap: check if name is null
  memzone: check result of rte_fbarray_get
  memzone: check result of malloc_elem_from_data
  malloc: malloc_elem_join_adjacent_free can return null
  malloc: check result of rte_mem_virt2memseg_list
  malloc: check result of rte_fbarray_get
  malloc: check result of rte_mem_virt2memseg
  malloc: check result of malloc_elem_free
  malloc: check result of elem_start_pt
  bus/vdev: check result of rte_vdev_device_name

 drivers/net/tap/rte_eth_tap.c        |  3 +++
 lib/eal/common/eal_common_memalloc.c |  5 ++++-
 lib/eal/common/eal_common_memzone.c  | 10 +++++++++-
 lib/eal/common/malloc_elem.c         | 14 +++++++++++---
 lib/eal/common/malloc_heap.c         |  9 ++++++++-
 lib/ethdev/ethdev_vdev.h             |  2 ++
 lib/ethdev/rte_class_eth.c           |  4 +++-
 7 files changed, 40 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH RESEND v2 01/11] ethdev: check return result of rte_eth_dev_info_get

[okaya]

From: Sinan Kaya <okaya@kernel.org>

rte_class_eth: eth_mac_cmp: The status of this call to rte_eth_dev_info_get
is not checked, potentially leaving dev_info uninitialized.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/ethdev/rte_class_eth.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/ethdev/rte_class_eth.c b/lib/ethdev/rte_class_eth.c
index 838b3a8f9f..8165e5adc0 100644
--- a/lib/ethdev/rte_class_eth.c
+++ b/lib/ethdev/rte_class_eth.c
@@ -51,7 +51,9 @@ eth_mac_cmp(const char *key __rte_unused,
 		return -1; /* invalid devargs value */
 
 	/* Return 0 if devargs MAC is matching one of the device MACs. */
-	rte_eth_dev_info_get(data->port_id, &dev_info);
+	if (rte_eth_dev_info_get(data->port_id, &dev_info) < 0)
+		return -1;
+
 	for (index = 0; index < dev_info.max_mac_addrs; index++)
 		if (rte_is_same_ether_addr(&mac, &data->mac_addrs[index]))
 			return 0;
-- 
2.25.1

[PATCH RESEND v2 02/11] net/tap: check if name is null

[okaya]

From: Sinan Kaya <okaya@kernel.org>

In rte_pmd_tun_probe result of call to rte_vdev_device_name is
dereferenced here and may be null.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 drivers/net/tap/rte_eth_tap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index f2a6c33a19..b99439e4f2 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -2340,6 +2340,9 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
 	struct rte_eth_dev *eth_dev;
 
 	name = rte_vdev_device_name(dev);
+	if (name == NULL)
+		return -1;
+
 	params = rte_vdev_device_args(dev);
 	memset(remote_iface, 0, RTE_ETH_NAME_MAX_LEN);
 
-- 
2.25.1

[PATCH RESEND v2 03/11] memzone: check result of rte_fbarray_get

[okaya]

From: Sinan Kaya <okaya@kernel.org>

In memzone_lookup_thread_unsafe result of call to rte_fbarray_get
is dereferenced here and may be null.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_memzone.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
index 860fb5fb64..8d472505eb 100644
--- a/lib/eal/common/eal_common_memzone.c
+++ b/lib/eal/common/eal_common_memzone.c
@@ -41,7 +41,7 @@ memzone_lookup_thread_unsafe(const char *name)
 	i = rte_fbarray_find_next_used(arr, 0);
 	while (i >= 0) {
 		mz = rte_fbarray_get(arr, i);
-		if (mz->addr != NULL &&
+		if ((mz != NULL) && (mz->addr != NULL) &&
 				!strncmp(name, mz->name, RTE_MEMZONE_NAMESIZE))
 			return mz;
 		i = rte_fbarray_find_next_used(arr, i + 1);
@@ -358,6 +358,10 @@ dump_memzone(const struct rte_memzone *mz, void *arg)
 	fprintf(f, "physical segments used:\n");
 	ms_idx = RTE_PTR_DIFF(mz->addr, msl->base_va) / page_sz;
 	ms = rte_fbarray_get(&msl->memseg_arr, ms_idx);
+	if (ms == NULL) {
+		RTE_LOG(DEBUG, EAL, "Skipping bad memzone\n");
+		return;
+	}
 
 	do {
 		fprintf(f, "  addr: %p iova: 0x%" PRIx64 " "
-- 
2.25.1

[PATCH RESEND v2 04/11] memzone: check result of malloc_elem_from_data

[okaya]

From: Sinan Kaya <okaya@kernel.org>

In memzone_reserve_aligned_thread_unsafe result of call
to malloc_elem_from_data is dereferenced here and may be null.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_memzone.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
index 8d472505eb..930fee5fdc 100644
--- a/lib/eal/common/eal_common_memzone.c
+++ b/lib/eal/common/eal_common_memzone.c
@@ -169,6 +169,10 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
 	}
 
 	struct malloc_elem *elem = malloc_elem_from_data(mz_addr);
+	if (elem == NULL) {
+		rte_errno = ENOSPC;
+		return NULL;
+	}
 
 	/* fill the zone in config */
 	mz_idx = rte_fbarray_find_next_free(arr, 0);
-- 
2.25.1

[PATCH RESEND v2 05/11] malloc: malloc_elem_join_adjacent_free can return null

[okaya]

From: Sinan Kaya <okaya@kernel.org>

In malloc_heap_add_memory result of call to malloc_elem_join_adjacent_free
is dereferenced here and may be null.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/malloc_heap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index d7c410b786..503e551bf9 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -97,6 +97,8 @@ malloc_heap_add_memory(struct malloc_heap *heap, struct rte_memseg_list *msl,
 	malloc_elem_insert(elem);
 
 	elem = malloc_elem_join_adjacent_free(elem);
+	if (elem == NULL)
+		return NULL;
 
 	malloc_elem_free_list_insert(elem);
 
-- 
2.25.1

[PATCH RESEND v2 06/11] malloc: check result of rte_mem_virt2memseg_list

[okaya]

From: Sinan Kaya <okaya@kernel.org>

In alloc_pages_on_heap result of call to rte_mem_virt2memseg_list
is dereferenced here and may be null.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/malloc_heap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 503e551bf9..3f41430e42 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -323,6 +323,8 @@ alloc_pages_on_heap(struct malloc_heap *heap, uint64_t pg_sz, size_t elt_size,
 
 	map_addr = ms[0]->addr;
 	msl = rte_mem_virt2memseg_list(map_addr);
+	if (msl == NULL)
+		return NULL;
 
 	/* check if we wanted contiguous memory but didn't get it */
 	if (contig && !eal_memalloc_is_contig(msl, map_addr, alloc_sz)) {
-- 
2.25.1

[PATCH RESEND v2 07/11] malloc: check result of rte_fbarray_get

[okaya]

From: Sinan Kaya <okaya@kernel.org>

In eal_memalloc_is_contig result of call to rte_fbarray_get
is dereferenced here and may be null.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_memalloc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/eal_common_memalloc.c b/lib/eal/common/eal_common_memalloc.c
index ab04479c1c..24506f8447 100644
--- a/lib/eal/common/eal_common_memalloc.c
+++ b/lib/eal/common/eal_common_memalloc.c
@@ -126,6 +126,9 @@ eal_memalloc_is_contig(const struct rte_memseg_list *msl, void *start,
 
 		/* skip first iteration */
 		ms = rte_fbarray_get(&msl->memseg_arr, start_seg);
+		if (ms == NULL)
+			return false;
+
 		cur = ms->iova;
 		expected = cur + pgsz;
 
@@ -137,7 +140,7 @@ eal_memalloc_is_contig(const struct rte_memseg_list *msl, void *start,
 				cur_seg++, expected += pgsz) {
 			ms = rte_fbarray_get(&msl->memseg_arr, cur_seg);
 
-			if (ms->iova != expected)
+			if ((ms != NULL) && (ms->iova != expected))
 				return false;
 		}
 	}
-- 
2.25.1

[PATCH RESEND v2 08/11] malloc: check result of rte_mem_virt2memseg

[okaya]

From: Sinan Kaya <okaya@kernel.org>

In malloc_elem_find_max_iova_contig result of call to rte_mem_virt2memseg
is dereferenced here and may be null.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/malloc_elem.c | 11 ++++++++---
 lib/eal/common/malloc_heap.c |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/eal/common/malloc_elem.c b/lib/eal/common/malloc_elem.c
index 83f05497cc..8f49812846 100644
--- a/lib/eal/common/malloc_elem.c
+++ b/lib/eal/common/malloc_elem.c
@@ -63,6 +63,8 @@ malloc_elem_find_max_iova_contig(struct malloc_elem *elem, size_t align)
 
 	cur_page = RTE_PTR_ALIGN_FLOOR(contig_seg_start, page_sz);
 	ms = rte_mem_virt2memseg(cur_page, elem->msl);
+	if (ms == NULL)
+		return 0;
 
 	/* do first iteration outside the loop */
 	page_end = RTE_PTR_ADD(cur_page, page_sz);
@@ -91,9 +93,12 @@ malloc_elem_find_max_iova_contig(struct malloc_elem *elem, size_t align)
 			 * we're not blowing past data end.
 			 */
 			ms = rte_mem_virt2memseg(contig_seg_start, elem->msl);
-			cur_page = ms->addr;
-			/* don't trigger another recalculation */
-			expected_iova = ms->iova;
+			if (ms != NULL) {
+				cur_page = ms->addr;
+
+				/* don't trigger another recalculation */
+				expected_iova = ms->iova;
+			}
 			continue;
 		}
 		/* cur_seg_end ends on a page boundary or on data end. if we're
diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 3f41430e42..88270ce4d2 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -930,7 +930,7 @@ malloc_heap_free(struct malloc_elem *elem)
 		const struct rte_memseg *tmp =
 				rte_mem_virt2memseg(aligned_start, msl);
 
-		if (tmp->flags & RTE_MEMSEG_FLAG_DO_NOT_FREE) {
+		if ((tmp != NULL) && (tmp->flags & RTE_MEMSEG_FLAG_DO_NOT_FREE)) {
 			/* this is an unfreeable segment, so move start */
 			aligned_start = RTE_PTR_ADD(tmp->addr, tmp->len);
 		}
-- 
2.25.1

[PATCH RESEND v2 09/11] malloc: check result of malloc_elem_free

[okaya]

From: Sinan Kaya <okaya@kernel.org>

In malloc_heap_free result of call to malloc_elem_free is dereferenced
here and may be null.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/malloc_heap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 88270ce4d2..6eb6fcda5e 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -892,6 +892,9 @@ malloc_heap_free(struct malloc_elem *elem)
 	/* anything after this is a bonus */
 	ret = 0;
 
+	if (elem == NULL)
+		goto free_unlock;
+
 	/* ...of which we can't avail if we are in legacy mode, or if this is an
 	 * externally allocated segment.
 	 */
-- 
2.25.1

[PATCH RESEND v2 10/11] malloc: check result of elem_start_pt

[okaya]

From: Sinan Kaya <okaya@kernel.org>

In malloc_elem_alloc result of call to elem_start_pt is dereferenced
here and may be null.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/malloc_elem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/eal/common/malloc_elem.c b/lib/eal/common/malloc_elem.c
index 8f49812846..26296f2dba 100644
--- a/lib/eal/common/malloc_elem.c
+++ b/lib/eal/common/malloc_elem.c
@@ -435,6 +435,9 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
 {
 	struct malloc_elem *new_elem = elem_start_pt(elem, size, align, bound,
 			contig);
+	if (new_elem == NULL)
+		return NULL;
+
 	const size_t old_elem_size = (uintptr_t)new_elem - (uintptr_t)elem;
 	const size_t trailer_size = elem->size - old_elem_size - size -
 		MALLOC_ELEM_OVERHEAD;
-- 
2.25.1

[PATCH RESEND v2 11/11] bus/vdev: check result of rte_vdev_device_name

[okaya]

From: Sinan Kaya <okaya@kernel.org>

In rte_eth_vdev_allocate result of call to rte_vdev_device_name is
dereferenced here and may be null.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/ethdev/ethdev_vdev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/ethdev/ethdev_vdev.h b/lib/ethdev/ethdev_vdev.h
index 364f140f91..6d94a65d97 100644
--- a/lib/ethdev/ethdev_vdev.h
+++ b/lib/ethdev/ethdev_vdev.h
@@ -34,6 +34,8 @@ rte_eth_vdev_allocate(struct rte_vdev_device *dev, size_t private_data_size)
 {
 	struct rte_eth_dev *eth_dev;
 	const char *name = rte_vdev_device_name(dev);
+	if (name == NULL)
+		return NULL;
 
 	eth_dev = rte_eth_dev_allocate(name);
 	if (!eth_dev)
-- 
2.25.1

Re: [PATCH RESEND v2 00/11] codeql fixes for various subsystems

[Sinan Kaya]

[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]

On Tue, 2022-11-22 at 10:30 -0500, okaya@kernel.org wrote:
> From: Sinan Kaya <okaya@kernel.org>
> Following up the codeql reported problems first submittedby Stephen
> Hemminger here:
> https://lore.kernel.org/all/20220527161210.77212d0b@hermes.local/t/
> 
> Posting a series of fixes about potential null pointer accesses.
> Changes from v1:- Remove braces around single line statements- use
> NULL comparisons
> Sinan Kaya (11):  ethdev: check return result of
> rte_eth_dev_info_get  net/tap: check if name is null  memzone: check
> result of rte_fbarray_get  memzone: check result of
> malloc_elem_from_data  malloc: malloc_elem_join_adjacent_free can
> return null  malloc: check result of
> rte_mem_virt2memseg_list  malloc: check result of
> rte_fbarray_get  malloc: check result of rte_mem_virt2memseg  malloc:
> check result of malloc_elem_free  malloc: check result of
> elem_start_pt  bus/vdev: check result of rte_vdev_device_name
>  drivers/net/tap/rte_eth_tap.c        |  3 +++
> lib/eal/common/eal_common_memalloc.c |  5 ++++-
> lib/eal/common/eal_common_memzone.c  | 10 +++++++++-
> lib/eal/common/malloc_elem.c         | 14 +++++++++++---
> lib/eal/common/malloc_heap.c         |  9 ++++++++-
> lib/ethdev/ethdev_vdev.h             |  2 ++
> lib/ethdev/rte_class_eth.c           |  4 +++- 7 files changed, 40
> insertions(+), 7 deletions(-)
> 

Checking to see if I need to cover any feedback.



[-- Attachment #2: Type: text/html, Size: 2287 bytes --]

Re: [PATCH RESEND v2 00/11] codeql fixes for various subsystems

[Dmitry Kozlyuk]

2022-12-13 11:43 (UTC-0500), Sinan Kaya:
> Checking to see if I need to cover any feedback.

There was some feedback to pre-resend v2 that is not visible on patchwork:

http://inbox.dpdk.org/dev/CAJFAV8yEe9awya3OVLuqavfW0MzonSQJQ-K+2q4WampmyrjEGg@mail.gmail.com/

Re: [PATCH RESEND v2 00/11] codeql fixes for various subsystems

[Sinan Kaya]

On Tue, 2022-12-13 at 19:59 +0300, Dmitry Kozlyuk wrote:
> 2022-12-13 11:43 (UTC-0500), Sinan Kaya:
> > Checking to see if I need to cover any feedback.
> 
> There was some feedback to pre-resend v2 that is not visible on
> patchwork:
> 
> http://inbox.dpdk.org/dev/CAJFAV8yEe9awya3OVLuqavfW0MzonSQJQ-K+2q4WampmyrjEGg@mail.gmail.com/
> 
> 

Yes, I sent it again with RESEND tag this time on this series.
Just checking to see if it went through this time.
I do see the RESEND series on patchwork.

Re: [PATCH RESEND v2 00/11] codeql fixes for various subsystems

[Sinan Kaya]

On Tue, 2022-12-13 at 12:02 -0500, Sinan Kaya wrote:
> On Tue, 2022-12-13 at 19:59 +0300, Dmitry Kozlyuk wrote:
> > 2022-12-13 11:43 (UTC-0500), Sinan Kaya:
> > > Checking to see if I need to cover any feedback.
> > 
> > There was some feedback to pre-resend v2 that is not visible on
> > patchwork:
> > 
> > http://inbox.dpdk.org/dev/CAJFAV8yEe9awya3OVLuqavfW0MzonSQJQ-K+2q4WampmyrjEGg@mail.gmail.com/
> > 
> > 
> > 
> 
> Yes, I sent it again with RESEND tag this time on this series.
> Just checking to see if it went through this time.
> I do see the RESEND series on patchwork.
> 
> 

I was planning to send the fixes in batches hoping this one to
merge first. I guess I'll post the remaning bugfixes using a
different series.

Re: [PATCH RESEND v2 01/11] ethdev: check return result of rte_eth_dev_info_get

[Thomas Monjalon]

22/11/2022 16:30, okaya@kernel.org:
> rte_class_eth: eth_mac_cmp: The status of this call to rte_eth_dev_info_get
> is not checked, potentially leaving dev_info uninitialized.
[...]
>  	/* Return 0 if devargs MAC is matching one of the device MACs. */
> -	rte_eth_dev_info_get(data->port_id, &dev_info);
> +	if (rte_eth_dev_info_get(data->port_id, &dev_info) < 0)
> +		return -1;
> +
>  	for (index = 0; index < dev_info.max_mac_addrs; index++)
>  		if (rte_is_same_ether_addr(&mac, &data->mac_addrs[index]))
>  			return 0;

Acked-by: Thomas Monjalon <thomas@monjalon.net>

You would get more attention if you Cc the maintainers.
You can use this git option: --cc-cmd devtools/get-maintainer.sh

Re: [PATCH RESEND v2 02/11] net/tap: check if name is null

[Ferruh Yigit]

On 11/22/2022 3:30 PM, okaya@kernel.org wrote:
> From: Sinan Kaya <okaya@kernel.org>
> 
> In rte_pmd_tun_probe result of call to rte_vdev_device_name is
> dereferenced here and may be null.
> 
> Signed-off-by: Sinan Kaya <okaya@kernel.org>
> ---
>  drivers/net/tap/rte_eth_tap.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index f2a6c33a19..b99439e4f2 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -2340,6 +2340,9 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
>  	struct rte_eth_dev *eth_dev;
>  
>  	name = rte_vdev_device_name(dev);
> +	if (name == NULL)
> +		return -1;
> +
>  	params = rte_vdev_device_args(dev);
>  	memset(remote_iface, 0, RTE_ETH_NAME_MAX_LEN);
>  

Yes, technically 'rte_vdev_device_name()' can return NULL, but call
stack is like this:

`
vdev_probe_all_drivers() //in 'vdev.c'
  name = rte_vdev_device_name(dev);
  if (vdev_parse(name, &driver))
    return -1;
  ret = driver->probe(dev); //it is 'rte_pmd_tun_probe()'
`

I assume this is highlighted by a tool but in practice
'rte_vdev_device_name()' should not return NULL, and there are many
other location this check is missing.

Although it doesn't hurt, I think we can skip the check since it is
unnecessary.

Re: [PATCH RESEND v2 02/11] net/tap: check if name is null

[Sinan Kaya]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

On Wed, 2023-01-18 at 10:57 +0000, Ferruh Yigit wrote:
> I assume this is highlighted by a tool but in practice
> 
> 'rte_vdev_device_name()' should not return NULL, and there are many
> 
> other location this check is missing.

That's correct. Warning was found by codeql static analysis tool. When
I search the codebase,
I do see a mixture of null checks and no checks for this function. We
can ignore this patch, if you
feel strongly against it.

[-- Attachment #2: Type: text/html, Size: 817 bytes --]

Re: [PATCH RESEND v2 01/11] ethdev: check return result of rte_eth_dev_info_get

[Sinan Kaya]

On Wed, 2023-01-18 at 09:42 +0100, Thomas Monjalon wrote:
> 22/11/2022 16:30, 
> okaya@kernel.org
> :
> 
> > rte_class_eth: eth_mac_cmp: The status of this call to
> > rte_eth_dev_info_get
> > is not checked, potentially leaving dev_info uninitialized.
> 
> [...]
> 
> >        /* Return 0 if devargs MAC is matching one of the device
> > MACs. */
> > -     rte_eth_dev_info_get(data->port_id, &dev_info);
> > +     if (rte_eth_dev_info_get(data->port_id, &dev_info) < 0)
> > +             return -1;
> > +
> >        for (index = 0; index < dev_info.max_mac_addrs; index++)
> >                if (rte_is_same_ether_addr(&mac, &data-
> > >mac_addrs[index]))
> >                        return 0;
> 
> 
> 
> Acked-by: Thomas Monjalon <
> thomas@monjalon.net
> >
> 
> 

Thanks.

> 
> You would get more attention if you Cc the maintainers.
> 
> You can use this git option: --cc-cmd devtools/get-maintainer.sh

Good to know that we have get-maintainer script.

Re: [PATCH RESEND v2 01/11] ethdev: check return result of rte_eth_dev_info_get

[Ferruh Yigit]

On 11/22/2022 3:30 PM, okaya@kernel.org wrote:
> From: Sinan Kaya <okaya@kernel.org>
> 
> rte_class_eth: eth_mac_cmp: The status of this call to rte_eth_dev_info_get
> is not checked, potentially leaving dev_info uninitialized.
> 
> Signed-off-by: Sinan Kaya <okaya@kernel.org>
> ---
>  lib/ethdev/rte_class_eth.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/ethdev/rte_class_eth.c b/lib/ethdev/rte_class_eth.c
> index 838b3a8f9f..8165e5adc0 100644
> --- a/lib/ethdev/rte_class_eth.c
> +++ b/lib/ethdev/rte_class_eth.c
> @@ -51,7 +51,9 @@ eth_mac_cmp(const char *key __rte_unused,
>  		return -1; /* invalid devargs value */
>  
>  	/* Return 0 if devargs MAC is matching one of the device MACs. */
> -	rte_eth_dev_info_get(data->port_id, &dev_info);
> +	if (rte_eth_dev_info_get(data->port_id, &dev_info) < 0)
> +		return -1;
> +
>  	for (index = 0; index < dev_info.max_mac_addrs; index++)
>  		if (rte_is_same_ether_addr(&mac, &data->mac_addrs[index]))
>  			return 0;

Minor issue, but before patch, comment about MAC address match applies
to block of code that gets dev_info and does MAC compare.
After patch, get dev_info is separate code block and comment doesn't
really apply to that block, although it is possible to understand what
comment is referring, still I think better to move it down to the block
that does MAC compare.