[iptables PATCH 1/2] Fix DEBUG build

[iptables PATCH 1/2] Fix DEBUG build

[Phil Sutter]

Fixed commit missed to update this conditional call to
nft_rule_print_save().

Fixes: 1e8ef6a584754 ("nft: family_ops: Pass nft_handle to 'rule_to_cs' callback")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-shared.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 78e422781723f..426765641cff6 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -998,7 +998,7 @@ bool nft_ipv46_rule_find(struct nft_handle *h, struct nftnl_rule *r, void *data)
 
 	DEBUGP("comparing with... ");
 #ifdef DEBUG_DEL
-	nft_rule_print_save(r, NFT_RULE_APPEND, 0);
+	nft_rule_print_save(h, r, NFT_RULE_APPEND, 0);
 #endif
 	if (!h->ops->is_same(cs, &this))
 		goto out;
-- 
2.24.0

[iptables PATCH 2/2] xtables-restore: Fix parser feed from line buffer

[Phil Sutter]

When called with --noflush, xtables-restore would trip over chain lines:
Parser uses strtok() to separate chain name, policy and counters which
inserts nul-chars into the source string. Therefore strlen() can't be
used anymore to find end of line. Fix this by caching line length before
calling xtables_restore_parse_line().

Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 .../testcases/ipt-restore/0010-noflush-new-chain_0     | 10 ++++++++++
 iptables/xtables-restore.c                             |  4 +++-
 2 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100755 iptables/tests/shell/testcases/ipt-restore/0010-noflush-new-chain_0

diff --git a/iptables/tests/shell/testcases/ipt-restore/0010-noflush-new-chain_0 b/iptables/tests/shell/testcases/ipt-restore/0010-noflush-new-chain_0
new file mode 100755
index 0000000000000..739e684a21183
--- /dev/null
+++ b/iptables/tests/shell/testcases/ipt-restore/0010-noflush-new-chain_0
@@ -0,0 +1,10 @@
+#!/bin/sh -e
+
+# assert input feed from buffer doesn't trip over
+# added nul-chars from parsing chain line.
+
+$XT_MULTI iptables-restore --noflush <<EOF
+*filter
+:foobar - [0:0]
+-A foobar -j ACCEPT
+COMMIT
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index 2f0fe7d439d94..dd907e0b8ddd5 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -327,10 +327,12 @@ void xtables_restore_parse(struct nft_handle *h,
 	line = 0;
 	ptr = preload_buffer;
 	while (*ptr) {
+		size_t len = strlen(ptr);
+
 		h->error.lineno = ++line;
 		DEBUGP("%s: buffered line %d: '%s'\n", __func__, line, ptr);
 		xtables_restore_parse_line(h, p, &state, ptr);
-		ptr += strlen(ptr) + 1;
+		ptr += len + 1;
 	}
 	if (*buffer) {
 		h->error.lineno = ++line;
-- 
2.24.0

Re: [iptables PATCH 2/2] xtables-restore: Fix parser feed from line buffer

[Pablo Neira Ayuso]

On Wed, Dec 04, 2019 at 10:06:06AM +0100, Phil Sutter wrote:
> When called with --noflush, xtables-restore would trip over chain lines:
> Parser uses strtok() to separate chain name, policy and counters which
> inserts nul-chars into the source string. Therefore strlen() can't be
> used anymore to find end of line. Fix this by caching line length before
> calling xtables_restore_parse_line().
> 
> Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation")
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [iptables PATCH 1/2] Fix DEBUG build

[Pablo Neira Ayuso]

On Wed, Dec 04, 2019 at 10:06:05AM +0100, Phil Sutter wrote:
> Fixed commit missed to update this conditional call to
> nft_rule_print_save().
> 
> Fixes: 1e8ef6a584754 ("nft: family_ops: Pass nft_handle to 'rule_to_cs' callback")
> Signed-off-by: Phil Sutter <phil@nwl.cc>

If you still find all this debugging useful.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Otherwise, remove the nft DEBUG is another option. IIRC those were
added at very early stage to fix a few issues with -D and -C commands.

Pick the one you prefer. Thanks!

Re: [iptables PATCH 1/2] Fix DEBUG build

[Phil Sutter]

Hi,

On Wed, Dec 04, 2019 at 06:49:27PM +0100, Pablo Neira Ayuso wrote:
> On Wed, Dec 04, 2019 at 10:06:05AM +0100, Phil Sutter wrote:
> > Fixed commit missed to update this conditional call to
> > nft_rule_print_save().
> > 
> > Fixes: 1e8ef6a584754 ("nft: family_ops: Pass nft_handle to 'rule_to_cs' callback")
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> 
> If you still find all this debugging useful.
> 
> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> Otherwise, remove the nft DEBUG is another option. IIRC those were
> added at very early stage to fix a few issues with -D and -C commands.
> 
> Pick the one you prefer. Thanks!

While it's definitely not as convenient as calling 'nft --debug=<foo>',
it's better than nothing. So I'm rather tempted to try and implement a
permanent debug output option although all the added jumps will probably
kill kubernetes. ;)

Cheers, Phil