Linux Netfilter development
 help / color / mirror / Atom feed
From: Thomas Haller <thaller@redhat.com>
To: NetFilter <netfilter-devel@vger.kernel.org>
Cc: Thomas Haller <thaller@redhat.com>
Subject: [PATCH nft 2/2] libnftables: move init-once guard inside xt_init()
Date: Tue, 19 Sep 2023 14:36:17 +0200	[thread overview]
Message-ID: <20230919123621.2770734-2-thaller@redhat.com> (raw)
In-Reply-To: <20230919123621.2770734-1-thaller@redhat.com>

A library should not restrict being used by multiple threads or make
assumptions about how it's being used. Hence a "init_once" pattern
without no locking is racy, a code smell and should be avoided.

Note that libxtables is full of global variables and when linking against
it, libnftables cannot be used from multiple threads either. That is not
easy to fix.

Move the ugliness of "init_once" away from nft_ctx_new(), so that the
problem is concentrated closer to libxtables.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/libnftables.c |  6 +-----
 src/xt.c          | 15 +++++++++++++--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/libnftables.c b/src/libnftables.c
index c34ee43de1fa..ed6b7fb5554c 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -191,15 +191,11 @@ void nft_ctx_clear_include_paths(struct nft_ctx *ctx)
 EXPORT_SYMBOL(nft_ctx_new);
 struct nft_ctx *nft_ctx_new(uint32_t flags)
 {
-	static bool init_once;
 	struct nft_ctx *ctx;
 
-	if (!init_once) {
-		init_once = true;
 #ifdef HAVE_LIBXTABLES
-		xt_init();
+	xt_init();
 #endif
-	}
 
 	ctx = xzalloc(sizeof(struct nft_ctx));
 	nft_init(ctx);
diff --git a/src/xt.c b/src/xt.c
index d774e07395a6..bb87e86e02af 100644
--- a/src/xt.c
+++ b/src/xt.c
@@ -361,7 +361,18 @@ static struct xtables_globals xt_nft_globals = {
 
 void xt_init(void)
 {
-	/* Default to IPv4, but this changes in runtime */
-	xtables_init_all(&xt_nft_globals, NFPROTO_IPV4);
+	static bool init_once;
+
+	if (!init_once) {
+		/* libxtables is full of global variables and cannot be used
+		 * concurrently by multiple threads. Hence, it's fine that the
+		 * "init_once" guard is not thread-safe either.
+		 * Don't link against xtables if you want thread safety.
+		 */
+		init_once = true;
+
+		/* Default to IPv4, but this changes in runtime */
+		xtables_init_all(&xt_nft_globals, NFPROTO_IPV4);
+	}
 }
 #endif
-- 
2.41.0


  reply	other threads:[~2023-09-19 12:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-19 12:36 [PATCH nft 1/2] libnftables: drop gmp_init() and mp_set_memory_functions() Thomas Haller
2023-09-19 12:36 ` Thomas Haller [this message]
2023-09-19 13:42   ` [PATCH nft 2/2] libnftables: move init-once guard inside xt_init() Pablo Neira Ayuso
2023-09-19 13:42 ` [PATCH nft 1/2] libnftables: drop gmp_init() and mp_set_memory_functions() Pablo Neira Ayuso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230919123621.2770734-2-thaller@redhat.com \
    --to=thaller@redhat.com \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox