diff mbox series

eal: fix calling cleanup function twice

Message ID 20230601160855.119801-1-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series eal: fix calling cleanup function twice |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Functional success Functional PASS
ci/intel-Testing fail Testing issues
ci/iol-abi-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-aarch-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS

Commit Message

Bruce Richardson June 1, 2023, 4:08 p.m. UTC 
  If an app calls rte_eal_cleanup() inside it's own code, then cleanup
could be called a second time automatically when the app exits. While
mostly harmless, we can avoid any potential issues by guaranteeing that
cleanup only gets called once, in the same way that eal_init only ever
gets called once.

Note: This patch only touches Linux and FreeBSD. Windows EAL does not
have run-once guard on the init function, so omitting it in the cleanup
function.

Fixes: aec9c13c5257 ("eal: add function to release internal resources")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/common/eal_common_debug.c |  4 +++-
 lib/eal/freebsd/eal.c             | 10 ++++++++++
 lib/eal/linux/eal.c               | 10 ++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

Comments

Thomas Monjalon June 5, 2023, 4:12 p.m. UTC | #1 
01/06/2023 18:08, Bruce Richardson:
> If an app calls rte_eal_cleanup() inside it's own code, then cleanup
> could be called a second time automatically when the app exits. While
> mostly harmless, we can avoid any potential issues by guaranteeing that
> cleanup only gets called once, in the same way that eal_init only ever
> gets called once.
> 
> Note: This patch only touches Linux and FreeBSD. Windows EAL does not
> have run-once guard on the init function, so omitting it in the cleanup
> function.
> 
> Fixes: aec9c13c5257 ("eal: add function to release internal resources")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks.

Please we should keep the same behavior in Linux, FreeBSD and Windows.
Is there any volunteer to add run-once guards in init and cleanup
for Windows EAL?
diff mbox series

Patch

diff --git a/lib/eal/common/eal_common_debug.c b/lib/eal/common/eal_common_debug.c
index dcb554af1e..9cac9c6390 100644
--- a/lib/eal/common/eal_common_debug.c
+++ b/lib/eal/common/eal_common_debug.c
@@ -4,10 +4,12 @@ 
 
 #include <stdarg.h>
 #include <stdlib.h>
+#include <errno.h>
 
 #include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_debug.h>
+#include <rte_errno.h>
 
 void
 __rte_panic(const char *funcname, const char *format, ...)
@@ -39,7 +41,7 @@  rte_exit(int exit_code, const char *format, ...)
 	rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap);
 	va_end(ap);
 
-	if (rte_eal_cleanup() != 0)
+	if (rte_eal_cleanup() != 0 && rte_errno != EALREADY)
 		RTE_LOG(CRIT, EAL,
 			"EAL could not release all resources\n");
 	exit(exit_code);
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index f125a0d3de..a2ce188c83 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -898,6 +898,16 @@  rte_eal_init(int argc, char **argv)
 int
 rte_eal_cleanup(void)
 {
+	static uint32_t run_once;
+	uint32_t has_run = 0;
+
+	if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
+			__ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
+		RTE_LOG(WARNING, EAL, "Already called cleanup\n");
+		rte_errno = EALREADY;
+		return -1;
+	}
+
 	struct internal_config *internal_conf =
 		eal_get_internal_configuration();
 	rte_service_finalize();
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index c80647d6b2..f8d66b9b6d 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1361,6 +1361,16 @@  mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 int
 rte_eal_cleanup(void)
 {
+	static uint32_t run_once;
+	uint32_t has_run = 0;
+
+	if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
+					__ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
+		RTE_LOG(WARNING, EAL, "Already called cleanup\n");
+		rte_errno = EALREADY;
+		return -1;
+	}
+
 	/* if we're in a primary process, we need to mark hugepages as freeable
 	 * so that finalization can release them back to the system.
 	 */