[iptables PATCH] ebtables: Fix for broken chain renaming

[iptables PATCH] ebtables: Fix for broken chain renaming

[Phil Sutter]

Loading extensions pollutes 'errno' value, hence before using it to
indicate failure it should be sanitized. This was done by the called
function before the parsing/netlink split and not migrated by accident.
Move it into calling code to clarify the connection.

Fixes: a7f1e208cdf9c ("nft: split parsing from netlink commands")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c                                                | 3 ---
 iptables/tests/shell/testcases/ebtables/0001-ebtables-basic_0 | 4 ++++
 iptables/xtables-eb.c                                         | 1 +
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 39882a443a974..411e2597205c9 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1896,9 +1896,6 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
 		return 0;
 	}
 
-	/* Config load changed errno. Ensure genuine info for our callers. */
-	errno = 0;
-
 	/* Find the old chain to be renamed */
 	c = nft_chain_find(h, table, chain);
 	if (c == NULL) {
diff --git a/iptables/tests/shell/testcases/ebtables/0001-ebtables-basic_0 b/iptables/tests/shell/testcases/ebtables/0001-ebtables-basic_0
index 0c1eb4ca66f52..6f11bd12593dd 100755
--- a/iptables/tests/shell/testcases/ebtables/0001-ebtables-basic_0
+++ b/iptables/tests/shell/testcases/ebtables/0001-ebtables-basic_0
@@ -86,4 +86,8 @@ if [ $? -eq 0 ]; then
 	exit 1
 fi
 
+$XT_MULTI ebtables -t filter -E FOO BAZ || exit 1
+$XT_MULTI ebtables -t filter -L | grep -q FOO && exit 1
+$XT_MULTI ebtables -t filter -L | grep -q BAZ || exit 1
+
 $XT_MULTI ebtables -t $t -F || exit 0
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index 6641a21a72d32..5e4184b8e80de 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -853,6 +853,7 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table,
 				else if (strchr(argv[optind], ' ') != NULL)
 					xtables_error(PARAMETER_PROBLEM, "Use of ' ' not allowed in chain names");
 
+				errno = 0;
 				ret = nft_cmd_chain_user_rename(h, chain, *table,
 							    argv[optind]);
 				if (ret != 0 && errno == ENOENT)
-- 
2.28.0

Re: [iptables PATCH] ebtables: Fix for broken chain renaming

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> Loading extensions pollutes 'errno' value, hence before using it to
> indicate failure it should be sanitized. This was done by the called
> function before the parsing/netlink split and not migrated by accident.
> Move it into calling code to clarify the connection.
> 
> Fixes: a7f1e208cdf9c ("nft: split parsing from netlink commands")
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Heh.  Thanks for adding a test -- LGTM, feel free to push this.

Re: [iptables PATCH] ebtables: Fix for broken chain renaming

[Phil Sutter]

Hi,

On Tue, Nov 17, 2020 at 12:07:25PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > Loading extensions pollutes 'errno' value, hence before using it to
> > indicate failure it should be sanitized. This was done by the called
> > function before the parsing/netlink split and not migrated by accident.
> > Move it into calling code to clarify the connection.
> > 
> > Fixes: a7f1e208cdf9c ("nft: split parsing from netlink commands")
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> 
> Heh.  Thanks for adding a test -- LGTM, feel free to push this.

DONE. Thanks for the quick review, I'll go fix Fedora now. :/

On Tue, Nov 17, 2020 at 12:08:04PM +0100, Pablo Neira Ayuso wrote:
> LGTM, this is fixing one recent netfilter's bugzilla ticket, right?

Oh, right! I noticed it because of a ticket for Fedora[1]. Will close
nfbz#1481 as well. Thanks for the heads-up!

Cheers, Phil

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1898130