netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Phil Sutter <phil@nwl.cc>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org,
	Arturo Borrero Gonzalez <arturo@netfilter.org>
Subject: [iptables PATCH] xtables-restore: fix for --noflush and empty lines
Date: Tue, 11 Feb 2020 18:09:13 +0100	[thread overview]
Message-ID: <20200211170913.2374-1-phil@nwl.cc> (raw)

Lookahead buffer used for cache requirements estimate in restore
--noflush separates individual lines with nul-chars. Two consecutive
nul-chars are interpreted as end of buffer and remaining buffer content
is skipped.

Sadly, reading an empty line (i.e., one containing a newline character
only) caused double nul-chars to appear in buffer as well, leading to
premature stop when reading cached lines from buffer.

To fix that, make use of xtables_restore_parse_line() skipping empty
lines without calling strtok() and just leave the newline character in
place. A more intuitive approach, namely skipping empty lines while
buffering, is deliberately not chosen as that would cause wrong values
in 'line' variable.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1400
Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 .../ipt-restore/0011-noflush-empty-line_0        | 16 ++++++++++++++++
 iptables/xtables-restore.c                       |  8 +++++---
 2 files changed, 21 insertions(+), 3 deletions(-)
 create mode 100755 iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0

diff --git a/iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0 b/iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0
new file mode 100755
index 0000000000000..bea1a690bb624
--- /dev/null
+++ b/iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0
@@ -0,0 +1,16 @@
+#!/bin/bash -e
+
+# make sure empty lines won't break --noflush
+
+cat <<EOF | $XT_MULTI iptables-restore --noflush
+# just a comment followed by innocent empty line
+
+*filter
+-A FORWARD -j ACCEPT
+COMMIT
+EOF
+
+EXPECT='Chain FORWARD (policy ACCEPT)
+target     prot opt source               destination         
+ACCEPT     all  --  0.0.0.0/0            0.0.0.0/0           '
+diff -u <(echo "$EXPECT") <($XT_MULTI iptables -n -L FORWARD)
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index 63cc15cee9621..fb2ac8b5c12a3 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -293,11 +293,13 @@ void xtables_restore_parse(struct nft_handle *h,
 		while (fgets(buffer, sizeof(buffer), p->in)) {
 			size_t blen = strlen(buffer);
 
-			/* drop trailing newline; xtables_restore_parse_line()
+			/* Drop trailing newline; xtables_restore_parse_line()
 			 * uses strtok() which replaces them by nul-characters,
 			 * causing unpredictable string delimiting in
-			 * preload_buffer */
-			if (buffer[blen - 1] == '\n')
+			 * preload_buffer.
+			 * Unless this is an empty line which would fold into a
+			 * spurious EoB indicator (double nul-char). */
+			if (buffer[blen - 1] == '\n' && blen > 1)
 				buffer[blen - 1] = '\0';
 			else
 				blen++;
-- 
2.24.1


             reply	other threads:[~2020-02-11 17:09 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-11 17:09 Phil Sutter [this message]
2020-02-12  9:21 ` [iptables PATCH] xtables-restore: fix for --noflush and empty lines Arturo Borrero Gonzalez

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=20200211170913.2374-1-phil@nwl.cc \
    --to=phil@nwl.cc \
    --cc=arturo@netfilter.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.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;
as well as URLs for NNTP newsgroup(s).