eal: fix calling cleanup function twice
Checks
Commit Message
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
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?
@@ -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);
@@ -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();
@@ -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.
*/