* [iptables PATCH] xtables-restore: fix for --noflush and empty lines
@ 2020-02-11 17:09 Phil Sutter
2020-02-12 9:21 ` Arturo Borrero Gonzalez
0 siblings, 1 reply; 2+ messages in thread
From: Phil Sutter @ 2020-02-11 17:09 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, Arturo Borrero Gonzalez
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [iptables PATCH] xtables-restore: fix for --noflush and empty lines
2020-02-11 17:09 [iptables PATCH] xtables-restore: fix for --noflush and empty lines Phil Sutter
@ 2020-02-12 9:21 ` Arturo Borrero Gonzalez
0 siblings, 0 replies; 2+ messages in thread
From: Arturo Borrero Gonzalez @ 2020-02-12 9:21 UTC (permalink / raw)
To: Phil Sutter, Pablo Neira Ayuso; +Cc: netfilter-devel
On 2/11/20 6:09 PM, Phil Sutter wrote:
> 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>
> ---
Thanks for working on this!
Acked-by: Arturo Borrero Gonzalez <arturo@netfilter.org>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-02-12 9:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-11 17:09 [iptables PATCH] xtables-restore: fix for --noflush and empty lines Phil Sutter
2020-02-12 9:21 ` Arturo Borrero Gonzalez
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).