nft: Support replacing a rule added in the same batch

As reported in nfbz#1820, trying to add a rule and replacing it in the
same batch would crash iptables due to a stale rule pointer left in an
obj_update.

Doing this is perfectly fine in legacy iptables, so implement the
missing feature instead of merely preventing the crash.

Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1820
Fixes: b199aca80da57 ("nft: Fix leak when replacing a rule")
Signed-off-by: Phil Sutter <phil@nwl.cc>

diff --git a/iptables/nft.c b/iptables/nft.c
index 908f544319b746aef74c2d9a30e11688f2036321..85080a6dc9ba786282e5bfdaeba4038a6672d60b 100644 (file)
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1764,6 +1764,31 @@ err:
        return NULL;
 }
 
        return NULL;
 }
 

+static struct obj_update *obj_update_by_rule(struct nft_handle *h,
+                                            struct nftnl_rule *r)
+{
+       struct obj_update *n;
+
+       list_for_each_entry(n, &h->obj_list, head) {
+               if (n->rule == r)
+                       return n;
+       }
+       return NULL;
+}
+
+static void copy_nftnl_rule_attr(struct nftnl_rule *to,
+                                const struct nftnl_rule *from,
+                                uint16_t attr)
+{
+       const void *data;
+       uint32_t len;
+
+       if (nftnl_rule_is_set(from, attr)) {
+               data = nftnl_rule_get_data(from, attr, &len);
+               nftnl_rule_set_data(to, attr, data, len);
+       }
+}
+

 int
 nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
                struct nftnl_rule *r, struct nftnl_rule *ref, bool verbose)
 int
 nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
                struct nftnl_rule *r, struct nftnl_rule *ref, bool verbose)


@@ -1775,12 +1800,31 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 
        nft_fn = nft_rule_append;
 
 
        nft_fn = nft_rule_append;
 

-       if (ref) {
-               nftnl_rule_set_u64(r, NFTNL_RULE_HANDLE,
-                                  nftnl_rule_get_u64(ref, NFTNL_RULE_HANDLE));
+       if (ref && !nftnl_rule_is_set(ref, NFTNL_RULE_HANDLE)) {
+               /* replacing a new rule, hijack its obj_update */
+               struct obj_update *n = obj_update_by_rule(h, ref);
+
+               if (!n) {
+                       errno = ENOENT;
+                       return 0;
+               }
+               if (n->type != NFT_COMPAT_RULE_APPEND &&
+                   n->type != NFT_COMPAT_RULE_INSERT) {
+                       errno = EINVAL;
+                       return 0;
+               }
+               copy_nftnl_rule_attr(r, ref, NFTNL_RULE_POSITION);
+               copy_nftnl_rule_attr(r, ref, NFTNL_RULE_ID);
+               nftnl_chain_rule_del(ref);
+               nftnl_rule_free(ref);
+               n->rule = r;
+               return 1;
+       } else if (ref) {
+               copy_nftnl_rule_attr(r, ref, NFTNL_RULE_HANDLE);

                type = NFT_COMPAT_RULE_REPLACE;
                type = NFT_COMPAT_RULE_REPLACE;

-       } else
+       } else {

                type = NFT_COMPAT_RULE_APPEND;
                type = NFT_COMPAT_RULE_APPEND;

+       }

 
        if (batch_rule_add(h, type, r) == NULL)
                return 0;
 
        if (batch_rule_add(h, type, r) == NULL)
                return 0;

diff --git a/iptables/tests/shell/testcases/ipt-restore/0018-replace-new_0 b/iptables/tests/shell/testcases/ipt-restore/0018-replace-new_0
new file mode 100755 (executable)
index 0000000..3930bde
--- /dev/null
+++ b/iptables/tests/shell/testcases/ipt-restore/0018-replace-new_0
@@ -0,0 +1,31 @@
+#!/bin/bash
+
+set -e
+
+RS='*filter
+-A FORWARD -m comment --comment "new rule being replaced"
+-R FORWARD 1 -m comment --comment "new replacing rule"
+COMMIT'
+EXP='*filter
+:INPUT ACCEPT [0:0]
+:FORWARD ACCEPT [0:0]
+:OUTPUT ACCEPT [0:0]
+-A FORWARD -m comment --comment "new replacing rule"
+COMMIT'
+$XT_MULTI iptables-restore <<< "$RS"
+diff -u -Z <(echo -e "$EXP") <($XT_MULTI iptables-save | grep -v '^#')
+
+RS='*filter
+-A FORWARD -m comment --comment "rule to insert before"
+-I FORWARD 1 -m comment --comment "new rule being replaced"
+-R FORWARD 1 -m comment --comment "new replacing rule"
+COMMIT'
+EXP='*filter
+:INPUT ACCEPT [0:0]
+:FORWARD ACCEPT [0:0]
+:OUTPUT ACCEPT [0:0]
+-A FORWARD -m comment --comment "new replacing rule"
+-A FORWARD -m comment --comment "rule to insert before"
+COMMIT'
+$XT_MULTI iptables-restore <<< "$RS"
+diff -u -Z <(echo -e "$EXP") <($XT_MULTI iptables-save | grep -v '^#')