RE: iptables patch

* RE: iptables patch
       [not found]   ` <AM9PR02MB7660795D89727FA09BD370DFA8889@AM9PR02MB7660.eurprd02.prod.outlook.com>
@ 2023-03-28  7:28     ` Kevin Peeters
  2023-03-28 15:55       ` Phil Sutter
  0 siblings, 1 reply; 5+ messages in thread
From: Kevin Peeters @ 2023-03-28  7:28 UTC (permalink / raw)
  To: netfilter-devel

Hello,

I am using the 'iptables' source code in one of my software projects. More in detail, I am calling libiptc and libxtables from my own software API to add/delete/... iptables firewall rules.

While developing, I bumped into one issue while using libxtables and made a patch for it which we now use on our checkout of the 'iptables' repository. We do however use multiple checkouts of this repository in different places and don't want to add the patch to each of those checkouts.
Would it be possible for you to add this patch to the mainline of your repository so we can stop patching it locally?

The details about the patch:
In libxtables/xtables.c:

The libxtables code uses a xtables_pending_matches, xtables_pending_targets, xtables_matches and xtables_targets pointer list to track all (pending) matches and targets registered to the current iptables command. In my code, I add/delete firewall rules multiple times from one main process (without killing the main process in between) by calling xtables_init_all, xtables_register_target and xtables_register_match every time. When a rule is added, I call xtables_fini to clean up.

I do notice when adding a rule in my code twice that on the second time, the (pending) targets/matches lists are not empty and when I try to register the same target (the one I registered in the previous rule) again, it links to itself and creates an infinite loop.

I managed to fix it by setting the pointers to NULL in xtables_fini.

The patch:

diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index 96fd783a..ac9300c7 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -327,6 +327,48 @@ void xtables_announce_chain(const char *name)
 		notargets_hlist_insert(name);
 }
 
+static void xtables_release_matches(void)
+{
+	struct xtables_match **dptr, **ptr;
+
+	for (dptr = &xtables_pending_matches; *dptr; ) {
+		ptr = &((*dptr)->next);
+		*dptr = NULL;
+		dptr = ptr;
+
+	}
+	xtables_pending_matches = NULL;
+
+	for (dptr = &xtables_matches; *dptr; ) {
+		ptr = &((*dptr)->next);
+		*dptr = NULL;
+		dptr = ptr;
+
+	}
+	xtables_matches = NULL;
+}
+
+static void xtables_release_targets(void)
+{
+	struct xtables_target **dptr, **ptr;
+
+	for (dptr = &xtables_pending_targets; *dptr; ) {
+		ptr = &((*dptr)->next);
+		*dptr = NULL;
+		dptr = ptr;
+
+	}
+	xtables_pending_targets = NULL;
+
+	for (dptr = &xtables_targets; *dptr; ) {
+		ptr = &((*dptr)->next);
+		*dptr = NULL;
+		dptr = ptr;
+
+	}
+	xtables_targets = NULL;
+}
+
 void xtables_init(void)
 {
 	/* xtables cannot be used with setuid in a safe way. */
@@ -366,6 +408,8 @@ void xtables_fini(void)
 	dlreg_free();
 #endif
 	notargets_hlist_free();
+	xtables_release_matches();
+	xtables_release_targets();
 }
 
 void xtables_set_nfproto(uint8_t nfproto)

Thanks in advance!

Kind regards,
Kevin Peeters

^ permalink raw reply related	[flat|nested] 5+ messages in thread