[PATCH 00/16] Fix allocation issues and add hardening

[PATCH 00/16] Fix allocation issues and add hardening

[Stephen Hemminger]

Recent versions of GCC have some additional function attributes that can
help with DPDK performance and stability.

The alloc_align attribute can tell the compiler what the alignment
of the allocation will be, and the optimizer can use this to produce
better code (especially memcpy and structure copies).

The malloc attribute tells compiler that object is not overlapping
and potentially aliasing. It also as an additional variant in GCC 11
or later that allows for detecting all sorts of common errors like
calling free() on memory allocated with rte_malloc().

This patch set is structured with:
  - add macros for enable the macros
  - fix any new warnings that were discovered
  - enable the attributes

The same attributes could be added to lots more functions in DPDK,
but this patch set focuses on the key ones, and where problems
exist in current code base.

The fixes should be backported to stable (they are real bugs),
but the macros common and the annotation in malloc should not.

Stephen Hemminger (16):
  eal: add function attributes for allocation functions
  memzone: fix use after free in tracing
  cryptodev/bcmfs: fix mis-matched free
  dma/ixd: fix incorrect free function in cleanup
  event/cnxk: fix pointer mismatch in cleanup
  examples/vhost: fix free function mismatch
  net/cnxk: fix use-after-free
  bpf: fix free mismatch if convert fails
  net/e1000: fix use-after-free
  net/sfc: fix use-after-free warning messages
  net/cpfl: fix free of nonheap object
  raw/ifpga/base: fix use after free
  common/qat: fix use after free
  drivers/ifpga: fix free function mismatch
  eal: add alloc_function attribute to rte_malloc
  mempool: annotate mempool create

 drivers/common/qat/qat_device.c           |  6 +--
 drivers/crypto/bcmfs/bcmfs_device.c       |  4 +-
 drivers/dma/idxd/idxd_pci.c               |  2 +-
 drivers/event/cnxk/cnxk_eventdev.c        |  4 +-
 drivers/net/cnxk/cnxk_ethdev_sec.c        |  2 +-
 drivers/net/cpfl/cpfl_flow_parser.c       |  1 -
 drivers/net/e1000/igb_ethdev.c            |  4 +-
 drivers/net/sfc/sfc_flow_rss.c            |  4 +-
 drivers/net/sfc/sfc_mae.c                 | 23 ++++-----
 drivers/raw/ifpga/base/opae_intel_max10.c | 11 +++-
 drivers/raw/ifpga/ifpga_rawdev.c          |  8 +--
 examples/vhost_blk/vhost_blk.c            |  2 +-
 lib/bpf/bpf_convert.c                     |  2 +-
 lib/eal/common/eal_common_memzone.c       |  3 +-
 lib/eal/include/rte_common.h              | 30 +++++++++++
 lib/eal/include/rte_malloc.h              | 63 ++++++++++++++---------
 lib/mempool/rte_mempool.h                 | 41 ++++++++-------
 17 files changed, 130 insertions(+), 80 deletions(-)

-- 
2.45.2

[PATCH 01/16] eal: add function attributes for allocation functions

[Stephen Hemminger]

The allocation functions take a alignment argument that
can be useful to hint the compiler optimizer.

This is supported by Gcc and Clang but only useful with
Gcc because Clang gives warning if alignment is 0.

Recent versions of GCC have a malloc attribute that can
be used to find mismatches between allocation and free;
the typical problem caught is a pointer allocated with
rte_malloc() that is then incorrectly freed using free().

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/include/rte_common.h | 30 ++++++++++++++++++++++++++++++
 lib/eal/include/rte_malloc.h | 24 ++++++++++++++++--------
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index eec0400dad..1b3781274d 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -228,6 +228,36 @@ typedef uint16_t unaligned_uint16_t;
 #define __rte_alloc_size(...)
 #endif
 
+/**
+ * Tells the compiler that the function returns a value that points to
+ * memory aligned by a function argument.
+ * Not enabled on clang because it warns if align argument is zero.
+ */
+#if defined(RTE_CC_GCC)
+#define __rte_alloc_align(align_arg) \
+	__attribute__((alloc_align(align_arg)))
+#else
+#define __rte_alloc_align(...)
+#endif
+
+/**
+ * Tells the compiler this is a function like malloc and that the pointer
+ * returned cannot alias any other pointer (ie new memory).
+ *
+ * Also, with recent GCC versions also able to track that proper
+ * dealloctor function is used for this pointer.
+ */
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 110000)
+#define __rte_alloc_func(free_func) \
+	__attribute__((malloc, malloc(free_func)))
+
+#elif defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
+#define __rte_alloc_func(free_func) \
+	__attribute__((malloc))
+#else
+#define __rte_alloc_func(free_func)
+#endif
+
 #define RTE_PRIORITY_LOG 101
 #define RTE_PRIORITY_BUS 110
 #define RTE_PRIORITY_CLASS 120
diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h
index 1f91e7bdde..cf3c174022 100644
--- a/lib/eal/include/rte_malloc.h
+++ b/lib/eal/include/rte_malloc.h
@@ -54,7 +54,8 @@ struct rte_malloc_socket_stats {
  */
 void *
 rte_malloc(const char *type, size_t size, unsigned align)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * Allocate zeroed memory from the heap.
@@ -81,7 +82,8 @@ rte_malloc(const char *type, size_t size, unsigned align)
  */
 void *
 rte_zmalloc(const char *type, size_t size, unsigned align)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * Replacement function for calloc(), using huge-page memory. Memory area is
@@ -108,7 +110,8 @@ rte_zmalloc(const char *type, size_t size, unsigned align)
  */
 void *
 rte_calloc(const char *type, size_t num, size_t size, unsigned align)
-	__rte_alloc_size(2, 3);
+	__rte_alloc_size(2, 3)
+	__rte_alloc_align(4);
 
 /**
  * Replacement function for realloc(), using huge-page memory. Reserved area
@@ -132,7 +135,8 @@ rte_calloc(const char *type, size_t num, size_t size, unsigned align)
  */
 void *
 rte_realloc(void *ptr, size_t size, unsigned int align)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * Replacement function for realloc(), using huge-page memory. Reserved area
@@ -158,7 +162,8 @@ rte_realloc(void *ptr, size_t size, unsigned int align)
  */
 void *
 rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * This function allocates memory from the huge-page area of memory. The memory
@@ -185,7 +190,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
  */
 void *
 rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * Allocate zeroed memory from the heap.
@@ -214,7 +220,8 @@ rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
  */
 void *
 rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * Replacement function for calloc(), using huge-page memory. Memory area is
@@ -243,7 +250,8 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
  */
 void *
 rte_calloc_socket(const char *type, size_t num, size_t size, unsigned align, int socket)
-	__rte_alloc_size(2, 3);
+	__rte_alloc_size(2, 3)
+	__rte_alloc_align(4);
 
 /**
  * Frees the memory space pointed to by the provided pointer.
-- 
2.45.2

[PATCH 02/16] memzone: fix use after free in tracing

[Stephen Hemminger]

Using the freed value for tracing is not a good idea.
Although it is harmless for tracing, it will cause analyzers to flag
this as unsafe.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_memzone.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
index 2d9b6aa3e3..90efbb621d 100644
--- a/lib/eal/common/eal_common_memzone.c
+++ b/lib/eal/common/eal_common_memzone.c
@@ -331,9 +331,10 @@ rte_memzone_free(const struct rte_memzone *mz)
 
 	rte_rwlock_write_unlock(&mcfg->mlock);
 
+	rte_eal_trace_memzone_free(name, addr, ret);
+
 	rte_free(addr);
 
-	rte_eal_trace_memzone_free(name, addr, ret);
 	return ret;
 }
 
-- 
2.45.2

[PATCH 03/16] cryptodev/bcmfs: fix mis-matched free

[Stephen Hemminger]

The device structure is allocated with rte_malloc() and
then incorrectly freed with free(). This will lead to
corrupt malloc pool.

Bugzilla ID: 1552
Fixes: c8e79da7c676 ("crypto/bcmfs: introduce BCMFS driver")
Cc: vikas.gupta@broadcom.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/crypto/bcmfs/bcmfs_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/bcmfs/bcmfs_device.c b/drivers/crypto/bcmfs/bcmfs_device.c
index ada7ba342c..46522970d5 100644
--- a/drivers/crypto/bcmfs/bcmfs_device.c
+++ b/drivers/crypto/bcmfs/bcmfs_device.c
@@ -139,7 +139,7 @@ fsdev_allocate_one_dev(struct rte_vdev_device *vdev,
 	return fsdev;
 
 cleanup:
-	free(fsdev);
+	rte_free(fsdev);
 
 	return NULL;
 }
@@ -163,7 +163,7 @@ fsdev_release(struct bcmfs_device *fsdev)
 		return;
 
 	TAILQ_REMOVE(&fsdev_list, fsdev, next);
-	free(fsdev);
+	rte_free(fsdev);
 }
 
 static int
-- 
2.45.2

[PATCH 04/16] dma/ixd: fix incorrect free function in cleanup

[Stephen Hemminger]

The data structure is allocated with rte_malloc and incorrectly
freed in cleanup logic using free.

Bugzilla ID: 1549
Fixes: 9449330a8458 ("dma/idxd: create dmadev instances on PCI probe")
Cc: kevin.laatz@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/dma/idxd/idxd_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/idxd_pci.c b/drivers/dma/idxd/idxd_pci.c
index 81637d9420..f89e2b41ff 100644
--- a/drivers/dma/idxd/idxd_pci.c
+++ b/drivers/dma/idxd/idxd_pci.c
@@ -301,7 +301,7 @@ init_pci_device(struct rte_pci_device *dev, struct idxd_dmadev *idxd,
 	return nb_wqs;
 
 err:
-	free(pci);
+	rte_free(pci);
 	return err_code;
 }
 
-- 
2.45.2

[PATCH 05/16] event/cnxk: fix pointer mismatch in cleanup

[Stephen Hemminger]

The code to cleanup in case of error was passing incorrect
value to rte_free. The ports[] entry was allocated with
rte_malloc and that should be used instead of the offset
in that object.

Fixes: 97a05c1fe634 ("event/cnxk: add port config")
Cc: sthotton@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/event/cnxk/cnxk_eventdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/event/cnxk/cnxk_eventdev.c b/drivers/event/cnxk/cnxk_eventdev.c
index 4b2d6bffa6..08c6ce0c07 100644
--- a/drivers/event/cnxk/cnxk_eventdev.c
+++ b/drivers/event/cnxk/cnxk_eventdev.c
@@ -121,8 +121,10 @@ cnxk_setup_event_ports(const struct rte_eventdev *event_dev,
 	return 0;
 hws_fini:
 	for (i = i - 1; i >= 0; i--) {
+		void *ws = event_dev->data->ports[i];
+
 		event_dev->data->ports[i] = NULL;
-		rte_free(cnxk_sso_hws_get_cookie(event_dev->data->ports[i]));
+		rte_free(ws);
 	}
 	return -ENOMEM;
 }
-- 
2.45.2

[PATCH 06/16] examples/vhost: fix free function mismatch

[Stephen Hemminger]

The pointer bdev is allocated with rte_zmalloc() and then
incorrectly freed with free() which will lead pool corruption.

Bugzilla ID: 1553
Fixes: c19beb3f38cd ("examples/vhost_blk: introduce vhost storage sample")
Cc: jin.yu@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/vhost_blk/vhost_blk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/vhost_blk/vhost_blk.c b/examples/vhost_blk/vhost_blk.c
index 03f1ac9c3f..9c9e326949 100644
--- a/examples/vhost_blk/vhost_blk.c
+++ b/examples/vhost_blk/vhost_blk.c
@@ -776,7 +776,7 @@ vhost_blk_bdev_construct(const char *bdev_name,
 	bdev->data = rte_zmalloc(NULL, blk_cnt * blk_size, 0);
 	if (!bdev->data) {
 		fprintf(stderr, "No enough reserved huge memory for disk\n");
-		free(bdev);
+		rte_free(bdev);
 		return NULL;
 	}
 
-- 
2.45.2

[PATCH 07/16] net/cnxk: fix use-after-free

[Stephen Hemminger]

The driver would refer to the mempool object after it was freed.

Bugzilla ID: 1554
Fixes: 7ea187184a51 ("common/cnxk: support 1-N pool-aura per NIX LF")
Cc: rbhansali@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/cnxk/cnxk_ethdev_sec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/cnxk/cnxk_ethdev_sec.c b/drivers/net/cnxk/cnxk_ethdev_sec.c
index 6f5319e534..e428d2115d 100644
--- a/drivers/net/cnxk/cnxk_ethdev_sec.c
+++ b/drivers/net/cnxk/cnxk_ethdev_sec.c
@@ -136,8 +136,8 @@ cnxk_nix_inl_custom_meta_pool_cb(uintptr_t pmpool, uintptr_t *mpool, const char
 			return -EINVAL;
 		}
 
-		rte_mempool_free(hp);
 		plt_free(hp->pool_config);
+		rte_mempool_free(hp);
 
 		*aura_handle = 0;
 		*mpool = 0;
-- 
2.45.2

[PATCH 08/16] bpf: fix free mismatch if convert fails

[Stephen Hemminger]

If conversion of cBF to eBPF fails then an object allocated with
rte_malloc() would be passed to free().

[908/3201] Compiling C object lib/librte_bpf.a.p/bpf_bpf_convert.c.o
../lib/bpf/bpf_convert.c: In function ‘rte_bpf_convert’:
../lib/bpf/bpf_convert.c:559:17: warning: ‘free’ called on pointer returned from a mismatched allocation function [-Wmismatched-dealloc]
  559 |                 free(prm);
      |                 ^~~~~~~~~
../lib/bpf/bpf_convert.c:545:15: note: returned from ‘rte_zmalloc’
  545 |         prm = rte_zmalloc("bpf_filter",
      |               ^~~~~~~~~~~~~~~~~~~~~~~~~
  546 |                           sizeof(*prm) + ebpf_len * sizeof(*ebpf), 0);
      |                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/bpf/bpf_convert.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bpf/bpf_convert.c b/lib/bpf/bpf_convert.c
index d7ff2b4325..e7e298c9cb 100644
--- a/lib/bpf/bpf_convert.c
+++ b/lib/bpf/bpf_convert.c
@@ -556,7 +556,7 @@ rte_bpf_convert(const struct bpf_program *prog)
 	ret = bpf_convert_filter(prog->bf_insns, prog->bf_len, ebpf, &ebpf_len);
 	if (ret < 0) {
 		RTE_BPF_LOG_LINE(ERR, "%s: cannot convert cBPF to eBPF", __func__);
-		free(prm);
+		rte_free(prm);
 		rte_errno = -ret;
 		return NULL;
 	}
-- 
2.45.2

[PATCH 09/16] net/e1000: fix use-after-free

[Stephen Hemminger]

The driver cleanup code was freeing the filter object
then dereferencing it.

Bugzilla ID: 1550
Fixes: 6a4d050e2855 ("net/igb: flush all the filter")
Cc: wei.zhao1@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/e1000/igb_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 095be27b08..973d0d2407 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -3907,11 +3907,11 @@ igb_delete_2tuple_filter(struct rte_eth_dev *dev,
 
 	filter_info->twotuple_mask &= ~(1 << filter->index);
 	TAILQ_REMOVE(&filter_info->twotuple_list, filter, entries);
-	rte_free(filter);
 
 	E1000_WRITE_REG(hw, E1000_TTQF(filter->index), E1000_TTQF_DISABLE_MASK);
 	E1000_WRITE_REG(hw, E1000_IMIR(filter->index), 0);
 	E1000_WRITE_REG(hw, E1000_IMIREXT(filter->index), 0);
+	rte_free(filter);
 	return 0;
 }
 
@@ -4348,7 +4348,6 @@ igb_delete_5tuple_filter_82576(struct rte_eth_dev *dev,
 
 	filter_info->fivetuple_mask &= ~(1 << filter->index);
 	TAILQ_REMOVE(&filter_info->fivetuple_list, filter, entries);
-	rte_free(filter);
 
 	E1000_WRITE_REG(hw, E1000_FTQF(filter->index),
 			E1000_FTQF_VF_BP | E1000_FTQF_MASK);
@@ -4357,6 +4356,7 @@ igb_delete_5tuple_filter_82576(struct rte_eth_dev *dev,
 	E1000_WRITE_REG(hw, E1000_SPQF(filter->index), 0);
 	E1000_WRITE_REG(hw, E1000_IMIR(filter->index), 0);
 	E1000_WRITE_REG(hw, E1000_IMIREXT(filter->index), 0);
+	rte_free(filter);
 	return 0;
 }
 
-- 
2.45.2

[PATCH 10/16] net/sfc: fix use-after-free warning messages

[Stephen Hemminger]

If compiler detection of use-after-free is enabled then this drivers
debug messages will cause warnings. Change to move debug message
before the object is freed.

Bugzilla ID: 1551
Fixes: 55c1238246d5 ("net/sfc: add more debug messages to transfer flows")
Cc: ivan.malov@oktetlabs.ru
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/sfc/sfc_flow_rss.c |  4 ++--
 drivers/net/sfc/sfc_mae.c      | 23 +++++++++--------------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/net/sfc/sfc_flow_rss.c b/drivers/net/sfc/sfc_flow_rss.c
index e28c943335..8e2749833b 100644
--- a/drivers/net/sfc/sfc_flow_rss.c
+++ b/drivers/net/sfc/sfc_flow_rss.c
@@ -303,9 +303,9 @@ sfc_flow_rss_ctx_del(struct sfc_adapter *sa, struct sfc_flow_rss_ctx *ctx)
 
 	TAILQ_REMOVE(&flow_rss->ctx_list, ctx, entries);
 	rte_free(ctx->qid_offsets);
-	rte_free(ctx);
-
 	sfc_dbg(sa, "flow-rss: deleted ctx=%p", ctx);
+
+	rte_free(ctx);
 }
 
 static int
diff --git a/drivers/net/sfc/sfc_mae.c b/drivers/net/sfc/sfc_mae.c
index 60ff6d2181..8f74f10390 100644
--- a/drivers/net/sfc/sfc_mae.c
+++ b/drivers/net/sfc/sfc_mae.c
@@ -400,9 +400,8 @@ sfc_mae_outer_rule_del(struct sfc_adapter *sa,
 	efx_mae_match_spec_fini(sa->nic, rule->match_spec);
 
 	TAILQ_REMOVE(&mae->outer_rules, rule, entries);
-	rte_free(rule);
-
 	sfc_dbg(sa, "deleted outer_rule=%p", rule);
+	rte_free(rule);
 }
 
 static int
@@ -585,9 +584,8 @@ sfc_mae_mac_addr_del(struct sfc_adapter *sa, struct sfc_mae_mac_addr *mac_addr)
 	}
 
 	TAILQ_REMOVE(&mae->mac_addrs, mac_addr, entries);
-	rte_free(mac_addr);
-
 	sfc_dbg(sa, "deleted mac_addr=%p", mac_addr);
+	rte_free(mac_addr);
 }
 
 enum sfc_mae_mac_addr_type {
@@ -785,10 +783,10 @@ sfc_mae_encap_header_del(struct sfc_adapter *sa,
 	}
 
 	TAILQ_REMOVE(&mae->encap_headers, encap_header, entries);
+	sfc_dbg(sa, "deleted encap_header=%p", encap_header);
+
 	rte_free(encap_header->buf);
 	rte_free(encap_header);
-
-	sfc_dbg(sa, "deleted encap_header=%p", encap_header);
 }
 
 static int
@@ -983,9 +981,8 @@ sfc_mae_counter_del(struct sfc_adapter *sa, struct sfc_mae_counter *counter)
 	}
 
 	TAILQ_REMOVE(&mae->counters, counter, entries);
-	rte_free(counter);
-
 	sfc_dbg(sa, "deleted counter=%p", counter);
+	rte_free(counter);
 }
 
 static int
@@ -1165,9 +1162,8 @@ sfc_mae_action_set_del(struct sfc_adapter *sa,
 	sfc_mae_mac_addr_del(sa, action_set->src_mac_addr);
 	sfc_mae_counter_del(sa, action_set->counter);
 	TAILQ_REMOVE(&mae->action_sets, action_set, entries);
-	rte_free(action_set);
-
 	sfc_dbg(sa, "deleted action_set=%p", action_set);
+	rte_free(action_set);
 }
 
 static int
@@ -1401,10 +1397,10 @@ sfc_mae_action_set_list_del(struct sfc_adapter *sa,
 		sfc_mae_action_set_del(sa, action_set_list->action_sets[i]);
 
 	TAILQ_REMOVE(&mae->action_set_lists, action_set_list, entries);
+	sfc_dbg(sa, "deleted action_set_list=%p", action_set_list);
+
 	rte_free(action_set_list->action_sets);
 	rte_free(action_set_list);
-
-	sfc_dbg(sa, "deleted action_set_list=%p", action_set_list);
 }
 
 static int
@@ -1667,9 +1663,8 @@ sfc_mae_action_rule_del(struct sfc_adapter *sa,
 	sfc_mae_outer_rule_del(sa, rule->outer_rule);
 
 	TAILQ_REMOVE(&mae->action_rules, rule, entries);
-	rte_free(rule);
-
 	sfc_dbg(sa, "deleted action_rule=%p", rule);
+	rte_free(rule);
 }
 
 static int
-- 
2.45.2

[PATCH 11/16] net/cpfl: fix free of nonheap object

[Stephen Hemminger]

With proper annotation, GCC discovers that this driver is
calling rte_free() on an object that was not allocated
(it is part of array in another object).

In function ‘cpfl_flow_js_mr_layout’,
    inlined from ‘cpfl_flow_js_mr_action’ at ../drivers/net/cpfl/cpfl_flow_parser.c:848:9,
    inlined from ‘cpfl_flow_js_mod_rule’ at ../drivers/net/cpfl/cpfl_flow_parser.c:908:9,
    inlined from ‘cpfl_parser_init’ at ../drivers/net/cpfl/cpfl_flow_parser.c:932:8,
    inlined from ‘cpfl_parser_create’ at ../drivers/net/cpfl/cpfl_flow_parser.c:959:8:
../drivers/net/cpfl/cpfl_flow_parser.c:740:9: warning: ‘rte_free’ called on pointer ‘*parser.modifications’ with nonzero offset [28, 15479062120396] [-Wfree-nonheap-object]
  740 |         rte_free(js_mod->layout);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 6cc97c9971d7 ("net/cpfl: build action mapping rules from JSON")
Cc: wenjing.qiao@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/cpfl/cpfl_flow_parser.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/cpfl/cpfl_flow_parser.c b/drivers/net/cpfl/cpfl_flow_parser.c
index 40569ddc6f..30abaad7c8 100644
--- a/drivers/net/cpfl/cpfl_flow_parser.c
+++ b/drivers/net/cpfl/cpfl_flow_parser.c
@@ -737,7 +737,6 @@ cpfl_flow_js_mr_layout(json_t *ob_layouts, struct cpfl_flow_js_mr_action_mod *js
 	return 0;
 
 err:
-	rte_free(js_mod->layout);
 	return -EINVAL;
 }
 
-- 
2.45.2

[PATCH 12/16] raw/ifpga/base: fix use after free

[Stephen Hemminger]

The TAILQ_FOREACH() macro would refer to info after it
had been freed. Fix by introducing TAILQ_FOREACH_SAFE here.

Fixes: 4a19f89104f8 ("raw/ifpga/base: support multiple cards")
Cc: tianfei.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/raw/ifpga/base/opae_intel_max10.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/raw/ifpga/base/opae_intel_max10.c b/drivers/raw/ifpga/base/opae_intel_max10.c
index dd97a5f9fd..d5a9ceb6e3 100644
--- a/drivers/raw/ifpga/base/opae_intel_max10.c
+++ b/drivers/raw/ifpga/base/opae_intel_max10.c
@@ -6,6 +6,13 @@
 #include <libfdt.h>
 #include "opae_osdep.h"
 
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
+	for ((var) = TAILQ_FIRST((head)); \
+		(var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
+	(var) = (tvar))
+#endif
+
 int max10_sys_read(struct intel_max10_device *dev,
 	unsigned int offset, unsigned int *val)
 {
@@ -746,9 +753,9 @@ static int fdt_get_named_reg(const void *fdt, int node, const char *name,
 
 static void max10_sensor_uinit(struct intel_max10_device *dev)
 {
-	struct opae_sensor_info *info;
+	struct opae_sensor_info *info, *next;
 
-	TAILQ_FOREACH(info, &dev->opae_sensor_list, node) {
+	TAILQ_FOREACH_SAFE(info, &dev->opae_sensor_list, node, next) {
 		TAILQ_REMOVE(&dev->opae_sensor_list, info, node);
 		opae_free(info);
 	}
-- 
2.45.2

[PATCH 13/16] common/qat: fix use after free

[Stephen Hemminger]

Checking return value of rte_memzone_free() is pointless
and if it failed then it was because the pointer was null.

Fixes: 7b1374b1e6e7 ("common/qat: limit configuration to primary process")
Cc: arkadiuszx.kusztal@intel.com

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/common/qat/qat_device.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/common/qat/qat_device.c b/drivers/common/qat/qat_device.c
index 4a972a83bd..bca88fd9bd 100644
--- a/drivers/common/qat/qat_device.c
+++ b/drivers/common/qat/qat_device.c
@@ -390,11 +390,7 @@ qat_pci_device_allocate(struct rte_pci_device *pci_dev)
 	return qat_dev;
 error:
 	rte_free(qat_dev->command_line);
-	if (rte_memzone_free(qat_dev_mz)) {
-		QAT_LOG(DEBUG,
-			"QAT internal error! Trying to free already allocated memzone: %s",
-			qat_dev_mz->name);
-	}
+	rte_memzone_free(qat_dev_mz);
 	return NULL;
 }
 
-- 
2.45.2

[PATCH 14/16] drivers/ifpga: fix free function mismatch

[Stephen Hemminger]

The raw ifpga driver redefines malloc to be opae_malloc
and free to be opae_free; which is a bad idea.

This leads to case where interrupt efd array is allocated
with calloc() and then passed to rte_free. The workaround
is to allocate the array with rte_calloc() instead.

Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle")
Cc: hkalra@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index a972b3b7a4..86558c7b9b 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1499,7 +1499,7 @@ ifpga_register_msix_irq(struct ifpga_rawdev *dev, int port_id,
 
 		nb_intr = rte_intr_nb_intr_get(*intr_handle);
 
-		intr_efds = calloc(nb_intr, sizeof(int));
+		intr_efds = rte_calloc("ifpga_efds", nb_intr, sizeof(int), 0);
 		if (!intr_efds)
 			return -ENOMEM;
 
@@ -1508,7 +1508,7 @@ ifpga_register_msix_irq(struct ifpga_rawdev *dev, int port_id,
 
 		ret = opae_acc_set_irq(acc, vec_start, count, intr_efds);
 		if (ret) {
-			free(intr_efds);
+			rte_free(intr_efds);
 			return -EINVAL;
 		}
 	}
@@ -1517,13 +1517,13 @@ ifpga_register_msix_irq(struct ifpga_rawdev *dev, int port_id,
 	ret = rte_intr_callback_register(*intr_handle,
 			handler, (void *)arg);
 	if (ret) {
-		free(intr_efds);
+		rte_free(intr_efds);
 		return -EINVAL;
 	}
 
 	IFPGA_RAWDEV_PMD_INFO("success register %s interrupt\n", name);
 
-	free(intr_efds);
+	rte_free(intr_efds);
 	return 0;
 }
 
-- 
2.45.2

[PATCH 15/16] eal: add alloc_function attribute to rte_malloc

[Stephen Hemminger]

Use the GCC function attribute to detect cases where
memory is allocated with rte_malloc and freed incorrectly
with libc version of free (and vice versa). Also will detect
some other pointer mismatches.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/include/rte_malloc.h | 55 +++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h
index cf3c174022..9e60a36476 100644
--- a/lib/eal/include/rte_malloc.h
+++ b/lib/eal/include/rte_malloc.h
@@ -31,6 +31,22 @@ struct rte_malloc_socket_stats {
 	size_t heap_allocsz_bytes; /**< Total allocated bytes on heap */
 };
 
+
+/**
+ * Frees the memory space pointed to by the provided pointer.
+ *
+ * This pointer must have been returned by a previous call to
+ * rte_malloc(), rte_zmalloc(), rte_calloc() or rte_realloc(). The behaviour of
+ * rte_free() is undefined if the pointer does not match this requirement.
+ *
+ * If the pointer is NULL, the function does nothing.
+ *
+ * @param ptr
+ *   The pointer to memory to be freed.
+ */
+void
+rte_free(void *ptr);
+
 /**
  * This function allocates memory from the huge-page area of memory. The memory
  * is not cleared. In NUMA systems, the memory allocated resides on the same
@@ -55,7 +71,8 @@ struct rte_malloc_socket_stats {
 void *
 rte_malloc(const char *type, size_t size, unsigned align)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free);
 
 /**
  * Allocate zeroed memory from the heap.
@@ -83,7 +100,8 @@ rte_malloc(const char *type, size_t size, unsigned align)
 void *
 rte_zmalloc(const char *type, size_t size, unsigned align)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free);
 
 /**
  * Replacement function for calloc(), using huge-page memory. Memory area is
@@ -111,7 +129,8 @@ rte_zmalloc(const char *type, size_t size, unsigned align)
 void *
 rte_calloc(const char *type, size_t num, size_t size, unsigned align)
 	__rte_alloc_size(2, 3)
-	__rte_alloc_align(4);
+	__rte_alloc_align(4)
+	__rte_alloc_func(rte_free);
 
 /**
  * Replacement function for realloc(), using huge-page memory. Reserved area
@@ -136,7 +155,8 @@ rte_calloc(const char *type, size_t num, size_t size, unsigned align)
 void *
 rte_realloc(void *ptr, size_t size, unsigned int align)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free);
 
 /**
  * Replacement function for realloc(), using huge-page memory. Reserved area
@@ -163,7 +183,8 @@ rte_realloc(void *ptr, size_t size, unsigned int align)
 void *
 rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free);
 
 /**
  * This function allocates memory from the huge-page area of memory. The memory
@@ -191,7 +212,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
 void *
 rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free);
 
 /**
  * Allocate zeroed memory from the heap.
@@ -221,7 +243,8 @@ rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
 void *
 rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free);
 
 /**
  * Replacement function for calloc(), using huge-page memory. Memory area is
@@ -251,22 +274,8 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
 void *
 rte_calloc_socket(const char *type, size_t num, size_t size, unsigned align, int socket)
 	__rte_alloc_size(2, 3)
-	__rte_alloc_align(4);
-
-/**
- * Frees the memory space pointed to by the provided pointer.
- *
- * This pointer must have been returned by a previous call to
- * rte_malloc(), rte_zmalloc(), rte_calloc() or rte_realloc(). The behaviour of
- * rte_free() is undefined if the pointer does not match this requirement.
- *
- * If the pointer is NULL, the function does nothing.
- *
- * @param ptr
- *   The pointer to memory to be freed.
- */
-void
-rte_free(void *ptr);
+	__rte_alloc_align(4)
+	__rte_alloc_func(rte_free);
 
 /**
  * If malloc debug is enabled, check a memory block for header
-- 
2.45.2

[PATCH 16/16] mempool: annotate mempool create

[Stephen Hemminger]

Use rte_alloc_function annotation to catch mismatch errors
on memzone handling.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/mempool/rte_mempool.h | 41 +++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 7bdc92b812..912500ce4c 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -1012,6 +1012,20 @@ typedef void (rte_mempool_mem_cb_t)(struct rte_mempool *mp,
  */
 typedef void (rte_mempool_ctor_t)(struct rte_mempool *, void *);
 
+/**
+ * Free a mempool
+ *
+ * Unlink the mempool from global list, free the memory chunks, and all
+ * memory referenced by the mempool. The objects must not be used by
+ * other cores as they will be freed.
+ *
+ * @param mp
+ *   A pointer to the mempool structure.
+ *   If NULL then, the function does nothing.
+ */
+void
+rte_mempool_free(struct rte_mempool *mp);
+
 /**
  * Create a new mempool named *name* in memory.
  *
@@ -1091,11 +1105,12 @@ typedef void (rte_mempool_ctor_t)(struct rte_mempool *, void *);
  *    - ENOMEM - no appropriate memory area found in which to create memzone
  */
 struct rte_mempool *
-rte_mempool_create(const char *name, unsigned n, unsigned elt_size,
-		   unsigned cache_size, unsigned private_data_size,
+rte_mempool_create(const char *name, unsigned int n, unsigned int elt_size,
+		   unsigned int cache_size, unsigned int private_data_size,
 		   rte_mempool_ctor_t *mp_init, void *mp_init_arg,
 		   rte_mempool_obj_cb_t *obj_init, void *obj_init_arg,
-		   int socket_id, unsigned flags);
+		   int socket_id, unsigned int flags)
+	__rte_alloc_func(rte_mempool_free);
 
 /**
  * Create an empty mempool
@@ -1132,22 +1147,10 @@ rte_mempool_create(const char *name, unsigned n, unsigned elt_size,
  *   with rte_errno set appropriately. See rte_mempool_create() for details.
  */
 struct rte_mempool *
-rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
-	unsigned cache_size, unsigned private_data_size,
-	int socket_id, unsigned flags);
-/**
- * Free a mempool
- *
- * Unlink the mempool from global list, free the memory chunks, and all
- * memory referenced by the mempool. The objects must not be used by
- * other cores as they will be freed.
- *
- * @param mp
- *   A pointer to the mempool structure.
- *   If NULL then, the function does nothing.
- */
-void
-rte_mempool_free(struct rte_mempool *mp);
+rte_mempool_create_empty(const char *name, unsigned int n, unsigned int elt_size,
+			 unsigned int cache_size, unsigned int private_data_size,
+			 int socket_id, unsigned int flags)
+	__rte_alloc_func(rte_mempool_free);
 
 /**
  * Add physically contiguous memory for objects in the pool at init
-- 
2.45.2

Re: [PATCH 01/16] eal: add function attributes for allocation functions

[David Marchand]

On Fri, Sep 27, 2024 at 4:48 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> The allocation functions take a alignment argument that
> can be useful to hint the compiler optimizer.
>
> This is supported by Gcc and Clang but only useful with
> Gcc because Clang gives warning if alignment is 0.
>
> Recent versions of GCC have a malloc attribute that can
> be used to find mismatches between allocation and free;
> the typical problem caught is a pointer allocated with
> rte_malloc() that is then incorrectly freed using free().

Interesting tool.

>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/eal/include/rte_common.h | 30 ++++++++++++++++++++++++++++++
>  lib/eal/include/rte_malloc.h | 24 ++++++++++++++++--------
>  2 files changed, 46 insertions(+), 8 deletions(-)
>
> diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> index eec0400dad..1b3781274d 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -228,6 +228,36 @@ typedef uint16_t unaligned_uint16_t;
>  #define __rte_alloc_size(...)
>  #endif
>
> +/**
> + * Tells the compiler that the function returns a value that points to
> + * memory aligned by a function argument.
> + * Not enabled on clang because it warns if align argument is zero.
> + */
> +#if defined(RTE_CC_GCC)
> +#define __rte_alloc_align(align_arg) \
> +       __attribute__((alloc_align(align_arg)))
> +#else
> +#define __rte_alloc_align(...)
> +#endif
> +
> +/**
> + * Tells the compiler this is a function like malloc and that the pointer
> + * returned cannot alias any other pointer (ie new memory).
> + *
> + * Also, with recent GCC versions also able to track that proper
> + * dealloctor function is used for this pointer.
> + */
> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 110000)

Even though it is probably equivalent, GCC_VERSION is set with RTE_CC_GCC.

> +#define __rte_alloc_func(free_func) \
> +       __attribute__((malloc, malloc(free_func)))

I read that this malloc attribute can also make use of the arg index
to assume the pointer is freed.

Did you try this feature?

Something like:

@@ -248,14 +248,13 @@ typedef uint16_t unaligned_uint16_t;
  * dealloctor function is used for this pointer.
  */
 #if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 110000)
-#define __rte_alloc_func(free_func) \
-       __attribute__((malloc, malloc(free_func)))
-
+#define __rte_alloc_func(...) \
+       __attribute__((malloc, malloc(__VA_ARGS__)))
 #elif defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
-#define __rte_alloc_func(free_func) \
+#define __rte_alloc_func(...) \
        __attribute__((malloc))
 #else
-#define __rte_alloc_func(free_func)
+#define __rte_alloc_func(...)
 #endif

 #define RTE_PRIORITY_LOG 101

> +
> +#elif defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
> +#define __rte_alloc_func(free_func) \
> +       __attribute__((malloc))
> +#else
> +#define __rte_alloc_func(free_func)
> +#endif
> +
>  #define RTE_PRIORITY_LOG 101
>  #define RTE_PRIORITY_BUS 110
>  #define RTE_PRIORITY_CLASS 120


-- 
David Marchand

Re: [PATCH 01/16] eal: add function attributes for allocation functions

[Stephen Hemminger]

On Fri, 27 Sep 2024 18:09:22 -0400
David Marchand <david.marchand@redhat.com> wrote:

> On Fri, Sep 27, 2024 at 4:48 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > The allocation functions take a alignment argument that
> > can be useful to hint the compiler optimizer.
> >
> > This is supported by Gcc and Clang but only useful with
> > Gcc because Clang gives warning if alignment is 0.
> >
> > Recent versions of GCC have a malloc attribute that can
> > be used to find mismatches between allocation and free;
> > the typical problem caught is a pointer allocated with
> > rte_malloc() that is then incorrectly freed using free().  
> 
> Interesting tool.
> 
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  lib/eal/include/rte_common.h | 30 ++++++++++++++++++++++++++++++
> >  lib/eal/include/rte_malloc.h | 24 ++++++++++++++++--------
> >  2 files changed, 46 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> > index eec0400dad..1b3781274d 100644
> > --- a/lib/eal/include/rte_common.h
> > +++ b/lib/eal/include/rte_common.h
> > @@ -228,6 +228,36 @@ typedef uint16_t unaligned_uint16_t;
> >  #define __rte_alloc_size(...)
> >  #endif
> >
> > +/**
> > + * Tells the compiler that the function returns a value that points to
> > + * memory aligned by a function argument.
> > + * Not enabled on clang because it warns if align argument is zero.
> > + */
> > +#if defined(RTE_CC_GCC)
> > +#define __rte_alloc_align(align_arg) \
> > +       __attribute__((alloc_align(align_arg)))
> > +#else
> > +#define __rte_alloc_align(...)
> > +#endif
> > +
> > +/**
> > + * Tells the compiler this is a function like malloc and that the pointer
> > + * returned cannot alias any other pointer (ie new memory).
> > + *
> > + * Also, with recent GCC versions also able to track that proper
> > + * dealloctor function is used for this pointer.
> > + */
> > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 110000)  
> 
> Even though it is probably equivalent, GCC_VERSION is set with RTE_CC_GCC.
> 
> > +#define __rte_alloc_func(free_func) \
> > +       __attribute__((malloc, malloc(free_func)))  
> 
> I read that this malloc attribute can also make use of the arg index
> to assume the pointer is freed.
> 
> Did you try this feature?

Yes, but all DPDK functions use first arg, so not really that relevant

RE: [PATCH 16/16] mempool: annotate mempool create

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, 27 September 2024 22.46
> 
> Use rte_alloc_function annotation to catch mismatch errors
> on memzone handling.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Note for other reviewers:
rte_mempool_free() was intentionally moved up,
so __rte_alloc_func attribute can refer to it.

Reviewed-by: Morten Brørup <mb@smartsharesystems.com>

Re: [PATCH 10/16] net/sfc: fix use-after-free warning messages

[Ivan Malov]

Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>

Thank you.

On Fri, 27 Sep 2024, Stephen Hemminger wrote:

> If compiler detection of use-after-free is enabled then this drivers
> debug messages will cause warnings. Change to move debug message
> before the object is freed.
>
> Bugzilla ID: 1551
> Fixes: 55c1238246d5 ("net/sfc: add more debug messages to transfer flows")
> Cc: ivan.malov@oktetlabs.ru
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> drivers/net/sfc/sfc_flow_rss.c |  4 ++--
> drivers/net/sfc/sfc_mae.c      | 23 +++++++++--------------
> 2 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/sfc/sfc_flow_rss.c b/drivers/net/sfc/sfc_flow_rss.c
> index e28c943335..8e2749833b 100644
> --- a/drivers/net/sfc/sfc_flow_rss.c
> +++ b/drivers/net/sfc/sfc_flow_rss.c
> @@ -303,9 +303,9 @@ sfc_flow_rss_ctx_del(struct sfc_adapter *sa, struct sfc_flow_rss_ctx *ctx)
>
> 	TAILQ_REMOVE(&flow_rss->ctx_list, ctx, entries);
> 	rte_free(ctx->qid_offsets);
> -	rte_free(ctx);
> -
> 	sfc_dbg(sa, "flow-rss: deleted ctx=%p", ctx);
> +
> +	rte_free(ctx);
> }
>
> static int
> diff --git a/drivers/net/sfc/sfc_mae.c b/drivers/net/sfc/sfc_mae.c
> index 60ff6d2181..8f74f10390 100644
> --- a/drivers/net/sfc/sfc_mae.c
> +++ b/drivers/net/sfc/sfc_mae.c
> @@ -400,9 +400,8 @@ sfc_mae_outer_rule_del(struct sfc_adapter *sa,
> 	efx_mae_match_spec_fini(sa->nic, rule->match_spec);
>
> 	TAILQ_REMOVE(&mae->outer_rules, rule, entries);
> -	rte_free(rule);
> -
> 	sfc_dbg(sa, "deleted outer_rule=%p", rule);
> +	rte_free(rule);
> }
>
> static int
> @@ -585,9 +584,8 @@ sfc_mae_mac_addr_del(struct sfc_adapter *sa, struct sfc_mae_mac_addr *mac_addr)
> 	}
>
> 	TAILQ_REMOVE(&mae->mac_addrs, mac_addr, entries);
> -	rte_free(mac_addr);
> -
> 	sfc_dbg(sa, "deleted mac_addr=%p", mac_addr);
> +	rte_free(mac_addr);
> }
>
> enum sfc_mae_mac_addr_type {
> @@ -785,10 +783,10 @@ sfc_mae_encap_header_del(struct sfc_adapter *sa,
> 	}
>
> 	TAILQ_REMOVE(&mae->encap_headers, encap_header, entries);
> +	sfc_dbg(sa, "deleted encap_header=%p", encap_header);
> +
> 	rte_free(encap_header->buf);
> 	rte_free(encap_header);
> -
> -	sfc_dbg(sa, "deleted encap_header=%p", encap_header);
> }
>
> static int
> @@ -983,9 +981,8 @@ sfc_mae_counter_del(struct sfc_adapter *sa, struct sfc_mae_counter *counter)
> 	}
>
> 	TAILQ_REMOVE(&mae->counters, counter, entries);
> -	rte_free(counter);
> -
> 	sfc_dbg(sa, "deleted counter=%p", counter);
> +	rte_free(counter);
> }
>
> static int
> @@ -1165,9 +1162,8 @@ sfc_mae_action_set_del(struct sfc_adapter *sa,
> 	sfc_mae_mac_addr_del(sa, action_set->src_mac_addr);
> 	sfc_mae_counter_del(sa, action_set->counter);
> 	TAILQ_REMOVE(&mae->action_sets, action_set, entries);
> -	rte_free(action_set);
> -
> 	sfc_dbg(sa, "deleted action_set=%p", action_set);
> +	rte_free(action_set);
> }
>
> static int
> @@ -1401,10 +1397,10 @@ sfc_mae_action_set_list_del(struct sfc_adapter *sa,
> 		sfc_mae_action_set_del(sa, action_set_list->action_sets[i]);
>
> 	TAILQ_REMOVE(&mae->action_set_lists, action_set_list, entries);
> +	sfc_dbg(sa, "deleted action_set_list=%p", action_set_list);
> +
> 	rte_free(action_set_list->action_sets);
> 	rte_free(action_set_list);
> -
> -	sfc_dbg(sa, "deleted action_set_list=%p", action_set_list);
> }
>
> static int
> @@ -1667,9 +1663,8 @@ sfc_mae_action_rule_del(struct sfc_adapter *sa,
> 	sfc_mae_outer_rule_del(sa, rule->outer_rule);
>
> 	TAILQ_REMOVE(&mae->action_rules, rule, entries);
> -	rte_free(rule);
> -
> 	sfc_dbg(sa, "deleted action_rule=%p", rule);
> +	rte_free(rule);
> }
>
> static int
> -- 
> 2.45.2
>
>

[PATCH v2 00/16] Fix allocation bugs and add malloc hardening

[Stephen Hemminger]

Recent versions of Gcc have some additonal function attributes tha can
help with DPDK performance and stability.

The alloc_align attribute can tell the compiler what the alignment
of the allocation will be, and the optimizer can use this to produce
better code (especially memcpy and structure copies).

The malloc attribute tells compiler that object is not overlapping
and potentially aliasing. It also as an additional variant in Gcc 11
or later that allows for detecting all sorts of common errors like
calling free() on memory allocated with rte_malloc().

This patch set is structured with:
  - add macros for enable the macros
  - fix any new warnings that were discovered
  - enable the attributes

In order to use the malloc attribute the free function prototype
needs to be moved before the allocator/create function prototype
so that the malloc attribute can refer to it.

The same attributes could be added to lots more functions in DPDK,
but this patchset focuses on the key ones, and where problems
exist in current code base.

The fixes should be backported to stable (they are real bugs),
but the macros common and the annotation in malloc should not.

v2 - add release note
   - add fix for nfp device
   - drop mempool (will pick it up in later series)

Stephen Hemminger (16):
  eal: add function attributes for allocation functions
  memzone: fix use after free in tracing
  cryptodev/bcmfs: fix mis-matched free
  dma/ixd: fix incorrect free function in cleanup
  event/cnxk: fix pointer mismatch in cleanup
  examples/vhost: fix free function mismatch
  net/cnxk: fix use-after-free
  bpf: fix free mismatch if convert fails
  net/e1000: fix use-after-free
  net/sfc: fix use-after-free warning messages
  net/cpfl: fix free of nonheap object
  net/nfp: fix duplicate call to rte_free
  raw/ifpga/base: fix use after free
  common/qat: fix use after free
  drivers/ifpga: fix free function mismatch
  eal: add alloc_function attribute to rte_malloc

 doc/guides/rel_notes/release_24_11.rst    |  8 +++
 drivers/common/qat/qat_device.c           |  6 +--
 drivers/crypto/bcmfs/bcmfs_device.c       |  4 +-
 drivers/dma/idxd/idxd_pci.c               |  2 +-
 drivers/event/cnxk/cnxk_eventdev.c        |  4 +-
 drivers/net/cnxk/cnxk_ethdev_sec.c        |  2 +-
 drivers/net/cpfl/cpfl_flow_parser.c       |  1 -
 drivers/net/e1000/igb_ethdev.c            |  4 +-
 drivers/net/nfp/flower/nfp_flower_flow.c  |  1 -
 drivers/net/sfc/sfc_flow_rss.c            |  4 +-
 drivers/net/sfc/sfc_mae.c                 | 23 ++++-----
 drivers/raw/ifpga/base/opae_intel_max10.c | 11 +++-
 drivers/raw/ifpga/ifpga_rawdev.c          |  8 +--
 examples/vhost_blk/vhost_blk.c            |  2 +-
 lib/bpf/bpf_convert.c                     |  2 +-
 lib/eal/common/eal_common_memzone.c       |  3 +-
 lib/eal/include/rte_common.h              | 30 +++++++++++
 lib/eal/include/rte_malloc.h              | 63 ++++++++++++++---------
 18 files changed, 116 insertions(+), 62 deletions(-)

-- 
2.45.2

[PATCH v2 01/16] eal: add function attributes for allocation functions

[Stephen Hemminger]

The allocation functions take a alignment argument that
can be useful to hint the compiler optimizer.

This is supported by Gcc and Clang but only useful with
Gcc because Clang gives warning if alignment is 0.

Recent versions of GCC have a malloc attribute that can
be used to find mismatches between allocation and free;
the typical problem caught is a pointer allocated with
rte_malloc() that is then incorrectly freed using free().

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/include/rte_common.h | 30 ++++++++++++++++++++++++++++++
 lib/eal/include/rte_malloc.h | 24 ++++++++++++++++--------
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index eec0400dad..e73c9f2aef 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -228,6 +228,36 @@ typedef uint16_t unaligned_uint16_t;
 #define __rte_alloc_size(...)
 #endif
 
+/**
+ * Tells the compiler that the function returns a value that points to
+ * memory aligned by a function argument.
+ * Not enabled on clang because it warns if align argument is zero.
+ */
+#if defined(RTE_CC_GCC)
+#define __rte_alloc_align(align_arg) \
+	__attribute__((alloc_align(align_arg)))
+#else
+#define __rte_alloc_align(...)
+#endif
+
+/**
+ * Tells the compiler this is a function like malloc and that the pointer
+ * returned cannot alias any other pointer (ie new memory).
+ *
+ * Also, with recent GCC versions also able to track that proper
+ * dealloctor function is used for this pointer.
+ */
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 110000)
+#define __rte_alloc_func(...) \
+	__attribute__((malloc, malloc(__VA_ARGS__)))
+
+#elif defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
+#define __rte_alloc_func(...) \
+	__attribute__((malloc))
+#else
+#define __rte_alloc_func(...)
+#endif
+
 #define RTE_PRIORITY_LOG 101
 #define RTE_PRIORITY_BUS 110
 #define RTE_PRIORITY_CLASS 120
diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h
index 1f91e7bdde..cf3c174022 100644
--- a/lib/eal/include/rte_malloc.h
+++ b/lib/eal/include/rte_malloc.h
@@ -54,7 +54,8 @@ struct rte_malloc_socket_stats {
  */
 void *
 rte_malloc(const char *type, size_t size, unsigned align)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * Allocate zeroed memory from the heap.
@@ -81,7 +82,8 @@ rte_malloc(const char *type, size_t size, unsigned align)
  */
 void *
 rte_zmalloc(const char *type, size_t size, unsigned align)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * Replacement function for calloc(), using huge-page memory. Memory area is
@@ -108,7 +110,8 @@ rte_zmalloc(const char *type, size_t size, unsigned align)
  */
 void *
 rte_calloc(const char *type, size_t num, size_t size, unsigned align)
-	__rte_alloc_size(2, 3);
+	__rte_alloc_size(2, 3)
+	__rte_alloc_align(4);
 
 /**
  * Replacement function for realloc(), using huge-page memory. Reserved area
@@ -132,7 +135,8 @@ rte_calloc(const char *type, size_t num, size_t size, unsigned align)
  */
 void *
 rte_realloc(void *ptr, size_t size, unsigned int align)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * Replacement function for realloc(), using huge-page memory. Reserved area
@@ -158,7 +162,8 @@ rte_realloc(void *ptr, size_t size, unsigned int align)
  */
 void *
 rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * This function allocates memory from the huge-page area of memory. The memory
@@ -185,7 +190,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
  */
 void *
 rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * Allocate zeroed memory from the heap.
@@ -214,7 +220,8 @@ rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
  */
 void *
 rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
-	__rte_alloc_size(2);
+	__rte_alloc_size(2)
+	__rte_alloc_align(3);
 
 /**
  * Replacement function for calloc(), using huge-page memory. Memory area is
@@ -243,7 +250,8 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
  */
 void *
 rte_calloc_socket(const char *type, size_t num, size_t size, unsigned align, int socket)
-	__rte_alloc_size(2, 3);
+	__rte_alloc_size(2, 3)
+	__rte_alloc_align(4);
 
 /**
  * Frees the memory space pointed to by the provided pointer.
-- 
2.45.2

[PATCH v2 02/16] memzone: fix use after free in tracing

[Stephen Hemminger]

Using the freed value for tracing is not a good idea.
Although it is harmless for tracing, it will cause analyzers to flag
this as unsafe.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_memzone.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
index 2d9b6aa3e3..90efbb621d 100644
--- a/lib/eal/common/eal_common_memzone.c
+++ b/lib/eal/common/eal_common_memzone.c
@@ -331,9 +331,10 @@ rte_memzone_free(const struct rte_memzone *mz)
 
 	rte_rwlock_write_unlock(&mcfg->mlock);
 
+	rte_eal_trace_memzone_free(name, addr, ret);
+
 	rte_free(addr);
 
-	rte_eal_trace_memzone_free(name, addr, ret);
 	return ret;
 }
 
-- 
2.45.2

[PATCH v2 03/16] cryptodev/bcmfs: fix mis-matched free

[Stephen Hemminger]

The device structure is allocated with rte_malloc() and
then incorrectly freed with free(). This will lead to
corrupt malloc pool.

Bugzilla ID: 1552
Fixes: c8e79da7c676 ("crypto/bcmfs: introduce BCMFS driver")
Cc: vikas.gupta@broadcom.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/crypto/bcmfs/bcmfs_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/bcmfs/bcmfs_device.c b/drivers/crypto/bcmfs/bcmfs_device.c
index ada7ba342c..46522970d5 100644
--- a/drivers/crypto/bcmfs/bcmfs_device.c
+++ b/drivers/crypto/bcmfs/bcmfs_device.c
@@ -139,7 +139,7 @@ fsdev_allocate_one_dev(struct rte_vdev_device *vdev,
 	return fsdev;
 
 cleanup:
-	free(fsdev);
+	rte_free(fsdev);
 
 	return NULL;
 }
@@ -163,7 +163,7 @@ fsdev_release(struct bcmfs_device *fsdev)
 		return;
 
 	TAILQ_REMOVE(&fsdev_list, fsdev, next);
-	free(fsdev);
+	rte_free(fsdev);
 }
 
 static int
-- 
2.45.2

[PATCH v2 04/16] dma/ixd: fix incorrect free function in cleanup

[Stephen Hemminger]

The data structure is allocated with rte_malloc and incorrectly
freed in cleanup logic using free.

Bugzilla ID: 1549
Fixes: 9449330a8458 ("dma/idxd: create dmadev instances on PCI probe")
Cc: kevin.laatz@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/dma/idxd/idxd_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/idxd_pci.c b/drivers/dma/idxd/idxd_pci.c
index 81637d9420..f89e2b41ff 100644
--- a/drivers/dma/idxd/idxd_pci.c
+++ b/drivers/dma/idxd/idxd_pci.c
@@ -301,7 +301,7 @@ init_pci_device(struct rte_pci_device *dev, struct idxd_dmadev *idxd,
 	return nb_wqs;
 
 err:
-	free(pci);
+	rte_free(pci);
 	return err_code;
 }
 
-- 
2.45.2

[PATCH v2 05/16] event/cnxk: fix pointer mismatch in cleanup

[Stephen Hemminger]

The code to cleanup in case of error was passing incorrect
value to rte_free. The ports[] entry was allocated with
rte_malloc and that should be used instead of the offset
in that object.

Fixes: 97a05c1fe634 ("event/cnxk: add port config")
Cc: sthotton@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/event/cnxk/cnxk_eventdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/event/cnxk/cnxk_eventdev.c b/drivers/event/cnxk/cnxk_eventdev.c
index 4b2d6bffa6..08c6ce0c07 100644
--- a/drivers/event/cnxk/cnxk_eventdev.c
+++ b/drivers/event/cnxk/cnxk_eventdev.c
@@ -121,8 +121,10 @@ cnxk_setup_event_ports(const struct rte_eventdev *event_dev,
 	return 0;
 hws_fini:
 	for (i = i - 1; i >= 0; i--) {
+		void *ws = event_dev->data->ports[i];
+
 		event_dev->data->ports[i] = NULL;
-		rte_free(cnxk_sso_hws_get_cookie(event_dev->data->ports[i]));
+		rte_free(ws);
 	}
 	return -ENOMEM;
 }
-- 
2.45.2

[PATCH v2 06/16] examples/vhost: fix free function mismatch

[Stephen Hemminger]

The pointer bdev is allocated with rte_zmalloc() and then
incorrectly freed with free() which will lead pool corruption.

Bugzilla ID: 1553
Fixes: c19beb3f38cd ("examples/vhost_blk: introduce vhost storage sample")
Cc: jin.yu@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/vhost_blk/vhost_blk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/vhost_blk/vhost_blk.c b/examples/vhost_blk/vhost_blk.c
index 03f1ac9c3f..9c9e326949 100644
--- a/examples/vhost_blk/vhost_blk.c
+++ b/examples/vhost_blk/vhost_blk.c
@@ -776,7 +776,7 @@ vhost_blk_bdev_construct(const char *bdev_name,
 	bdev->data = rte_zmalloc(NULL, blk_cnt * blk_size, 0);
 	if (!bdev->data) {
 		fprintf(stderr, "No enough reserved huge memory for disk\n");
-		free(bdev);
+		rte_free(bdev);
 		return NULL;
 	}
 
-- 
2.45.2

[PATCH v2 07/16] net/cnxk: fix use-after-free

[Stephen Hemminger]

The driver would refer to the mempool object after it was freed.

Bugzilla ID: 1554
Fixes: 7ea187184a51 ("common/cnxk: support 1-N pool-aura per NIX LF")
Cc: rbhansali@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/cnxk/cnxk_ethdev_sec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/cnxk/cnxk_ethdev_sec.c b/drivers/net/cnxk/cnxk_ethdev_sec.c
index 6f5319e534..e428d2115d 100644
--- a/drivers/net/cnxk/cnxk_ethdev_sec.c
+++ b/drivers/net/cnxk/cnxk_ethdev_sec.c
@@ -136,8 +136,8 @@ cnxk_nix_inl_custom_meta_pool_cb(uintptr_t pmpool, uintptr_t *mpool, const char
 			return -EINVAL;
 		}
 
-		rte_mempool_free(hp);
 		plt_free(hp->pool_config);
+		rte_mempool_free(hp);
 
 		*aura_handle = 0;
 		*mpool = 0;
-- 
2.45.2

[PATCH v2 08/16] bpf: fix free mismatch if convert fails

[Stephen Hemminger]

If conversion of cBF to eBPF fails then an object allocated with
rte_malloc() would be passed to free().

[908/3201] Compiling C object lib/librte_bpf.a.p/bpf_bpf_convert.c.o
../lib/bpf/bpf_convert.c: In function ‘rte_bpf_convert’:
../lib/bpf/bpf_convert.c:559:17: warning: ‘free’ called on pointer returned from a mismatched allocation function [-Wmismatched-dealloc]
  559 |                 free(prm);
      |                 ^~~~~~~~~
../lib/bpf/bpf_convert.c:545:15: note: returned from ‘rte_zmalloc’
  545 |         prm = rte_zmalloc("bpf_filter",
      |               ^~~~~~~~~~~~~~~~~~~~~~~~~
  546 |                           sizeof(*prm) + ebpf_len * sizeof(*ebpf), 0);
      |                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/bpf/bpf_convert.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bpf/bpf_convert.c b/lib/bpf/bpf_convert.c
index d7ff2b4325..e7e298c9cb 100644
--- a/lib/bpf/bpf_convert.c
+++ b/lib/bpf/bpf_convert.c
@@ -556,7 +556,7 @@ rte_bpf_convert(const struct bpf_program *prog)
 	ret = bpf_convert_filter(prog->bf_insns, prog->bf_len, ebpf, &ebpf_len);
 	if (ret < 0) {
 		RTE_BPF_LOG_LINE(ERR, "%s: cannot convert cBPF to eBPF", __func__);
-		free(prm);
+		rte_free(prm);
 		rte_errno = -ret;
 		return NULL;
 	}
-- 
2.45.2

[PATCH v2 09/16] net/e1000: fix use-after-free

[Stephen Hemminger]

The driver cleanup code was freeing the filter object
then dereferencing it.

Bugzilla ID: 1550
Fixes: 6a4d050e2855 ("net/igb: flush all the filter")
Cc: wei.zhao1@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/e1000/igb_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 095be27b08..973d0d2407 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -3907,11 +3907,11 @@ igb_delete_2tuple_filter(struct rte_eth_dev *dev,
 
 	filter_info->twotuple_mask &= ~(1 << filter->index);
 	TAILQ_REMOVE(&filter_info->twotuple_list, filter, entries);
-	rte_free(filter);
 
 	E1000_WRITE_REG(hw, E1000_TTQF(filter->index), E1000_TTQF_DISABLE_MASK);
 	E1000_WRITE_REG(hw, E1000_IMIR(filter->index), 0);
 	E1000_WRITE_REG(hw, E1000_IMIREXT(filter->index), 0);
+	rte_free(filter);
 	return 0;
 }
 
@@ -4348,7 +4348,6 @@ igb_delete_5tuple_filter_82576(struct rte_eth_dev *dev,
 
 	filter_info->fivetuple_mask &= ~(1 << filter->index);
 	TAILQ_REMOVE(&filter_info->fivetuple_list, filter, entries);
-	rte_free(filter);
 
 	E1000_WRITE_REG(hw, E1000_FTQF(filter->index),
 			E1000_FTQF_VF_BP | E1000_FTQF_MASK);
@@ -4357,6 +4356,7 @@ igb_delete_5tuple_filter_82576(struct rte_eth_dev *dev,
 	E1000_WRITE_REG(hw, E1000_SPQF(filter->index), 0);
 	E1000_WRITE_REG(hw, E1000_IMIR(filter->index), 0);
 	E1000_WRITE_REG(hw, E1000_IMIREXT(filter->index), 0);
+	rte_free(filter);
 	return 0;
 }
 
-- 
2.45.2

[PATCH v2 10/16] net/sfc: fix use-after-free warning messages

[Stephen Hemminger]

If compiler detection of use-after-free is enabled then this drivers
debug messages will cause warnings. Change to move debug message
before the object is freed.

Bugzilla ID: 1551
Fixes: 55c1238246d5 ("net/sfc: add more debug messages to transfer flows")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
---
 drivers/net/sfc/sfc_flow_rss.c |  4 ++--
 drivers/net/sfc/sfc_mae.c      | 23 +++++++++--------------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/net/sfc/sfc_flow_rss.c b/drivers/net/sfc/sfc_flow_rss.c
index e28c943335..8e2749833b 100644
--- a/drivers/net/sfc/sfc_flow_rss.c
+++ b/drivers/net/sfc/sfc_flow_rss.c
@@ -303,9 +303,9 @@ sfc_flow_rss_ctx_del(struct sfc_adapter *sa, struct sfc_flow_rss_ctx *ctx)
 
 	TAILQ_REMOVE(&flow_rss->ctx_list, ctx, entries);
 	rte_free(ctx->qid_offsets);
-	rte_free(ctx);
-
 	sfc_dbg(sa, "flow-rss: deleted ctx=%p", ctx);
+
+	rte_free(ctx);
 }
 
 static int
diff --git a/drivers/net/sfc/sfc_mae.c b/drivers/net/sfc/sfc_mae.c
index 60ff6d2181..8f74f10390 100644
--- a/drivers/net/sfc/sfc_mae.c
+++ b/drivers/net/sfc/sfc_mae.c
@@ -400,9 +400,8 @@ sfc_mae_outer_rule_del(struct sfc_adapter *sa,
 	efx_mae_match_spec_fini(sa->nic, rule->match_spec);
 
 	TAILQ_REMOVE(&mae->outer_rules, rule, entries);
-	rte_free(rule);
-
 	sfc_dbg(sa, "deleted outer_rule=%p", rule);
+	rte_free(rule);
 }
 
 static int
@@ -585,9 +584,8 @@ sfc_mae_mac_addr_del(struct sfc_adapter *sa, struct sfc_mae_mac_addr *mac_addr)
 	}
 
 	TAILQ_REMOVE(&mae->mac_addrs, mac_addr, entries);
-	rte_free(mac_addr);
-
 	sfc_dbg(sa, "deleted mac_addr=%p", mac_addr);
+	rte_free(mac_addr);
 }
 
 enum sfc_mae_mac_addr_type {
@@ -785,10 +783,10 @@ sfc_mae_encap_header_del(struct sfc_adapter *sa,
 	}
 
 	TAILQ_REMOVE(&mae->encap_headers, encap_header, entries);
+	sfc_dbg(sa, "deleted encap_header=%p", encap_header);
+
 	rte_free(encap_header->buf);
 	rte_free(encap_header);
-
-	sfc_dbg(sa, "deleted encap_header=%p", encap_header);
 }
 
 static int
@@ -983,9 +981,8 @@ sfc_mae_counter_del(struct sfc_adapter *sa, struct sfc_mae_counter *counter)
 	}
 
 	TAILQ_REMOVE(&mae->counters, counter, entries);
-	rte_free(counter);
-
 	sfc_dbg(sa, "deleted counter=%p", counter);
+	rte_free(counter);
 }
 
 static int
@@ -1165,9 +1162,8 @@ sfc_mae_action_set_del(struct sfc_adapter *sa,
 	sfc_mae_mac_addr_del(sa, action_set->src_mac_addr);
 	sfc_mae_counter_del(sa, action_set->counter);
 	TAILQ_REMOVE(&mae->action_sets, action_set, entries);
-	rte_free(action_set);
-
 	sfc_dbg(sa, "deleted action_set=%p", action_set);
+	rte_free(action_set);
 }
 
 static int
@@ -1401,10 +1397,10 @@ sfc_mae_action_set_list_del(struct sfc_adapter *sa,
 		sfc_mae_action_set_del(sa, action_set_list->action_sets[i]);
 
 	TAILQ_REMOVE(&mae->action_set_lists, action_set_list, entries);
+	sfc_dbg(sa, "deleted action_set_list=%p", action_set_list);
+
 	rte_free(action_set_list->action_sets);
 	rte_free(action_set_list);
-
-	sfc_dbg(sa, "deleted action_set_list=%p", action_set_list);
 }
 
 static int
@@ -1667,9 +1663,8 @@ sfc_mae_action_rule_del(struct sfc_adapter *sa,
 	sfc_mae_outer_rule_del(sa, rule->outer_rule);
 
 	TAILQ_REMOVE(&mae->action_rules, rule, entries);
-	rte_free(rule);
-
 	sfc_dbg(sa, "deleted action_rule=%p", rule);
+	rte_free(rule);
 }
 
 static int
-- 
2.45.2

[PATCH v2 11/16] net/cpfl: fix free of nonheap object

[Stephen Hemminger]

With proper annotation, GCC discovers that this driver is
calling rte_free() on an object that was not allocated
(it is part of array in another object).

In function ‘cpfl_flow_js_mr_layout’,
    inlined from ‘cpfl_flow_js_mr_action’ at ../drivers/net/cpfl/cpfl_flow_parser.c:848:9,
    inlined from ‘cpfl_flow_js_mod_rule’ at ../drivers/net/cpfl/cpfl_flow_parser.c:908:9,
    inlined from ‘cpfl_parser_init’ at ../drivers/net/cpfl/cpfl_flow_parser.c:932:8,
    inlined from ‘cpfl_parser_create’ at ../drivers/net/cpfl/cpfl_flow_parser.c:959:8:
../drivers/net/cpfl/cpfl_flow_parser.c:740:9: warning: ‘rte_free’ called on pointer ‘*parser.modifications’ with nonzero offset [28, 15479062120396] [-Wfree-nonheap-object]
  740 |         rte_free(js_mod->layout);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 6cc97c9971d7 ("net/cpfl: build action mapping rules from JSON")
Cc: wenjing.qiao@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/cpfl/cpfl_flow_parser.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/cpfl/cpfl_flow_parser.c b/drivers/net/cpfl/cpfl_flow_parser.c
index 40569ddc6f..30abaad7c8 100644
--- a/drivers/net/cpfl/cpfl_flow_parser.c
+++ b/drivers/net/cpfl/cpfl_flow_parser.c
@@ -737,7 +737,6 @@ cpfl_flow_js_mr_layout(json_t *ob_layouts, struct cpfl_flow_js_mr_action_mod *js
 	return 0;
 
 err:
-	rte_free(js_mod->layout);
 	return -EINVAL;
 }
 
-- 
2.45.2

[PATCH v2 12/16] net/nfp: fix duplicate call to rte_free

[Stephen Hemminger]

Calling rte_free twice on same object will corrupt the heap.
Warning is:
In function 'nfp_pre_tun_table_check_del',
inlined from 'nfp_flow_destroy' at ../drivers/net/nfp/flower/nfp_flower_flow.c:5143:9:
../drivers/net/nfp/flower/nfp_flower_flow.c:3830:9: error: pointer 'entry' used after 'rte_free' [-Werror=use-after-free]
3830 |         rte_free(entry);
|         ^~~~~~~~~~~~~~~
../drivers/net/nfp/flower/nfp_flower_flow.c:3825:9: note: call to 'rte_free' here
3825 |         rte_free(entry);
|         ^~~~~~~~~~~~~~~

Bugzilla ID: 1555
Fixes: d3c33bdf1f18 ("net/nfp: prepare for IPv4 UDP tunnel decap flow action")
Cc: chaoyong.he@corigine.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfp/flower/nfp_flower_flow.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/nfp/flower/nfp_flower_flow.c b/drivers/net/nfp/flower/nfp_flower_flow.c
index 0078455658..64a0062c8b 100644
--- a/drivers/net/nfp/flower/nfp_flower_flow.c
+++ b/drivers/net/nfp/flower/nfp_flower_flow.c
@@ -3822,7 +3822,6 @@ nfp_pre_tun_table_check_del(struct nfp_flower_representor *repr,
 		goto free_entry;
 	}
 
-	rte_free(entry);
 	rte_free(find_entry);
 	priv->pre_tun_cnt--;
 
-- 
2.45.2

[PATCH v2 13/16] raw/ifpga/base: fix use after free

[Stephen Hemminger]

The TAILQ_FOREACH() macro would refer to info after it
had been freed. Fix by introducing TAILQ_FOREACH_SAFE here.

Fixes: 4a19f89104f8 ("raw/ifpga/base: support multiple cards")
Cc: tianfei.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/raw/ifpga/base/opae_intel_max10.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/raw/ifpga/base/opae_intel_max10.c b/drivers/raw/ifpga/base/opae_intel_max10.c
index dd97a5f9fd..d5a9ceb6e3 100644
--- a/drivers/raw/ifpga/base/opae_intel_max10.c
+++ b/drivers/raw/ifpga/base/opae_intel_max10.c
@@ -6,6 +6,13 @@
 #include <libfdt.h>
 #include "opae_osdep.h"
 
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
+	for ((var) = TAILQ_FIRST((head)); \
+		(var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
+	(var) = (tvar))
+#endif
+
 int max10_sys_read(struct intel_max10_device *dev,
 	unsigned int offset, unsigned int *val)
 {
@@ -746,9 +753,9 @@ static int fdt_get_named_reg(const void *fdt, int node, const char *name,
 
 static void max10_sensor_uinit(struct intel_max10_device *dev)
 {
-	struct opae_sensor_info *info;
+	struct opae_sensor_info *info, *next;
 
-	TAILQ_FOREACH(info, &dev->opae_sensor_list, node) {
+	TAILQ_FOREACH_SAFE(info, &dev->opae_sensor_list, node, next) {
 		TAILQ_REMOVE(&dev->opae_sensor_list, info, node);
 		opae_free(info);
 	}
-- 
2.45.2

[PATCH v2 14/16] common/qat: fix use after free

[Stephen Hemminger]

Checking return value of rte_memzone_free() is pointless
and if it failed then it was because the pointer was null.

Fixes: 7b1374b1e6e7 ("common/qat: limit configuration to primary process")
Cc: arkadiuszx.kusztal@intel.com

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/common/qat/qat_device.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/common/qat/qat_device.c b/drivers/common/qat/qat_device.c
index 4a972a83bd..bca88fd9bd 100644
--- a/drivers/common/qat/qat_device.c
+++ b/drivers/common/qat/qat_device.c
@@ -390,11 +390,7 @@ qat_pci_device_allocate(struct rte_pci_device *pci_dev)
 	return qat_dev;
 error:
 	rte_free(qat_dev->command_line);
-	if (rte_memzone_free(qat_dev_mz)) {
-		QAT_LOG(DEBUG,
-			"QAT internal error! Trying to free already allocated memzone: %s",
-			qat_dev_mz->name);
-	}
+	rte_memzone_free(qat_dev_mz);
 	return NULL;
 }
 
-- 
2.45.2

[PATCH v2 15/16] drivers/ifpga: fix free function mismatch

[Stephen Hemminger]

The raw ifpga driver redefines malloc to be opae_malloc
and free to be opae_free; which is a bad idea.

This leads to case where interrupt efd array is allocated
with calloc() and then passed to rte_free. The workaround
is to allocate the array with rte_calloc() instead.

Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle")
Cc: hkalra@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index a972b3b7a4..86558c7b9b 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1499,7 +1499,7 @@ ifpga_register_msix_irq(struct ifpga_rawdev *dev, int port_id,
 
 		nb_intr = rte_intr_nb_intr_get(*intr_handle);
 
-		intr_efds = calloc(nb_intr, sizeof(int));
+		intr_efds = rte_calloc("ifpga_efds", nb_intr, sizeof(int), 0);
 		if (!intr_efds)
 			return -ENOMEM;
 
@@ -1508,7 +1508,7 @@ ifpga_register_msix_irq(struct ifpga_rawdev *dev, int port_id,
 
 		ret = opae_acc_set_irq(acc, vec_start, count, intr_efds);
 		if (ret) {
-			free(intr_efds);
+			rte_free(intr_efds);
 			return -EINVAL;
 		}
 	}
@@ -1517,13 +1517,13 @@ ifpga_register_msix_irq(struct ifpga_rawdev *dev, int port_id,
 	ret = rte_intr_callback_register(*intr_handle,
 			handler, (void *)arg);
 	if (ret) {
-		free(intr_efds);
+		rte_free(intr_efds);
 		return -EINVAL;
 	}
 
 	IFPGA_RAWDEV_PMD_INFO("success register %s interrupt\n", name);
 
-	free(intr_efds);
+	rte_free(intr_efds);
 	return 0;
 }
 
-- 
2.45.2

[PATCH v2 16/16] eal: add alloc_function attribute to rte_malloc

[Stephen Hemminger]

Use the GCC function attribute to detect cases where
memory is allocated with rte_malloc and freed incorrectly
with libc version of free (and vice versa). Also will detect
some other pointer mismatches.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/rel_notes/release_24_11.rst |  8 ++++
 lib/eal/include/rte_malloc.h           | 55 +++++++++++++++-----------
 2 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/doc/guides/rel_notes/release_24_11.rst b/doc/guides/rel_notes/release_24_11.rst
index 0ff70d9057..f27a37eac4 100644
--- a/doc/guides/rel_notes/release_24_11.rst
+++ b/doc/guides/rel_notes/release_24_11.rst
@@ -55,6 +55,14 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Hardened rte_malloc and related functions.**
+
+  * Added function attributes to  ``rte_malloc`` and similar functions
+    that can catch some obvious bugs at compile time (with GCC 11.0 or later).
+    Examples: calling ``free()`` on pointer that was allocated with ``rte_malloc``
+    (and vice versa); freeing the same pointer twice in the same routine;
+    freeing an object that was not created by allocation; etc.
+
 
 Removed Items
 -------------
diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h
index cf3c174022..c7af96fcba 100644
--- a/lib/eal/include/rte_malloc.h
+++ b/lib/eal/include/rte_malloc.h
@@ -31,6 +31,22 @@ struct rte_malloc_socket_stats {
 	size_t heap_allocsz_bytes; /**< Total allocated bytes on heap */
 };
 
+
+/**
+ * Frees the memory space pointed to by the provided pointer.
+ *
+ * This pointer must have been returned by a previous call to
+ * rte_malloc(), rte_zmalloc(), rte_calloc() or rte_realloc(). The behaviour of
+ * rte_free() is undefined if the pointer does not match this requirement.
+ *
+ * If the pointer is NULL, the function does nothing.
+ *
+ * @param ptr
+ *   The pointer to memory to be freed.
+ */
+void
+rte_free(void *ptr);
+
 /**
  * This function allocates memory from the huge-page area of memory. The memory
  * is not cleared. In NUMA systems, the memory allocated resides on the same
@@ -55,7 +71,8 @@ struct rte_malloc_socket_stats {
 void *
 rte_malloc(const char *type, size_t size, unsigned align)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free, 1);
 
 /**
  * Allocate zeroed memory from the heap.
@@ -83,7 +100,8 @@ rte_malloc(const char *type, size_t size, unsigned align)
 void *
 rte_zmalloc(const char *type, size_t size, unsigned align)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free, 1);
 
 /**
  * Replacement function for calloc(), using huge-page memory. Memory area is
@@ -111,7 +129,8 @@ rte_zmalloc(const char *type, size_t size, unsigned align)
 void *
 rte_calloc(const char *type, size_t num, size_t size, unsigned align)
 	__rte_alloc_size(2, 3)
-	__rte_alloc_align(4);
+	__rte_alloc_align(4)
+	__rte_alloc_func(rte_free, 1);
 
 /**
  * Replacement function for realloc(), using huge-page memory. Reserved area
@@ -136,7 +155,8 @@ rte_calloc(const char *type, size_t num, size_t size, unsigned align)
 void *
 rte_realloc(void *ptr, size_t size, unsigned int align)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free, 1);
 
 /**
  * Replacement function for realloc(), using huge-page memory. Reserved area
@@ -163,7 +183,8 @@ rte_realloc(void *ptr, size_t size, unsigned int align)
 void *
 rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free, 1);
 
 /**
  * This function allocates memory from the huge-page area of memory. The memory
@@ -191,7 +212,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
 void *
 rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free, 1);
 
 /**
  * Allocate zeroed memory from the heap.
@@ -221,7 +243,8 @@ rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
 void *
 rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
 	__rte_alloc_size(2)
-	__rte_alloc_align(3);
+	__rte_alloc_align(3)
+	__rte_alloc_func(rte_free, 1);
 
 /**
  * Replacement function for calloc(), using huge-page memory. Memory area is
@@ -251,22 +274,8 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
 void *
 rte_calloc_socket(const char *type, size_t num, size_t size, unsigned align, int socket)
 	__rte_alloc_size(2, 3)
-	__rte_alloc_align(4);
-
-/**
- * Frees the memory space pointed to by the provided pointer.
- *
- * This pointer must have been returned by a previous call to
- * rte_malloc(), rte_zmalloc(), rte_calloc() or rte_realloc(). The behaviour of
- * rte_free() is undefined if the pointer does not match this requirement.
- *
- * If the pointer is NULL, the function does nothing.
- *
- * @param ptr
- *   The pointer to memory to be freed.
- */
-void
-rte_free(void *ptr);
+	__rte_alloc_align(4)
+	__rte_alloc_func(rte_free, 1);
 
 /**
  * If malloc debug is enabled, check a memory block for header
-- 
2.45.2