When comparing two rules with non-standard targets, differences in
targets' payloads wasn't respected.
The cause is a rather hideous one: Unlike xtables_find_match(),
xtables_find_target() did not care whether the found target was already
in use or not, so the same target instance was assigned to both rules
and therefore payload comparison happened over the same memory location.
With legacy iptables it is not possible to reuse a target: The only case
where two rules (i.e., iptables_command_state instances) could exist at
the same time is when comparing rules, but that's handled using libiptc.
Signed-off-by: Phil Sutter <phil@xxxxxx>
---
Changes since v1:
- Reapply onto changed patch 1.
---
iptables/nft-bridge.c | 9 +++++++++
iptables/nft-shared.c | 5 +++++
.../testcases/iptables/0005-delete-rules_0 | 7 +++++++
libxtables/xtables.c | 18 +++++++++++++++++-
4 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index b4265c8af4f70..5757bedcd8724 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -45,6 +45,15 @@ void ebt_cs_clean(struct iptables_command_state *cs)
free(m);
m = nm;
}
+
+ if (cs->target) {
+ free(cs->target->t);
+
+ if (cs->target == cs->target->next) {
+ free(cs->target);
+ cs->target = NULL;
+ }
+ }
}
static void ebt_print_mac(const unsigned char *mac)
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 2d4b8d557eb73..a72d414d78111 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -687,6 +687,11 @@ void nft_clear_iptables_command_state(struct iptables_command_state *cs)
if (cs->target) {
free(cs->target->t);
cs->target->t = NULL;
+
+ if (cs->target == cs->target->next) {
+ free(cs->target);
+ cs->target = NULL;
+ }
}
}
diff --git a/iptables/tests/shell/testcases/iptables/0005-delete-rules_0 b/iptables/tests/shell/testcases/iptables/0005-delete-rules_0
index 9312fd53c3437..5038cbce5a5cf 100755
--- a/iptables/tests/shell/testcases/iptables/0005-delete-rules_0
+++ b/iptables/tests/shell/testcases/iptables/0005-delete-rules_0
@@ -5,3 +5,10 @@
$XT_MULTI iptables -A FORWARD -i eth23 -o eth42 -j DROP
$XT_MULTI iptables -D FORWARD -i eth23 -o eth42 -j REJECT
[[ $? -eq 1 ]] || exit 1
+
+# test incorrect deletion of rules with deviating payload
+# in non-standard target
+
+$XT_MULTI iptables -A FORWARD -i eth23 -o eth42 -j MARK --set-mark 23
+$XT_MULTI iptables -D FORWARD -i eth23 -o eth42 -j MARK --set-mark 42
+[[ $? -eq 1 ]] || exit 1
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index ea9bb102c8eb4..895f6988eaf57 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -756,8 +756,24 @@ xtables_find_target(const char *name, enum xtables_tryload tryload)
}
for (ptr = xtables_targets; ptr; ptr = ptr->next) {
- if (extension_cmp(name, ptr->name, ptr->family))
+ if (extension_cmp(name, ptr->name, ptr->family)) {
+ struct xtables_target *clone;
+
+ /* First target of this type: */
+ if (ptr->t == NULL)
+ break;
+
+ /* Second and subsequent clones */
+ clone = xtables_malloc(sizeof(struct xtables_target));
+ memcpy(clone, ptr, sizeof(struct xtables_target));
+ clone->udata = NULL;
+ clone->tflags = 0;
+ /* This is a clone: */
+ clone->next = clone;
+
+ ptr = clone;
break;
+ }
}
#ifndef NO_SHARED_LIBS
--
2.20.1
