[nftables PATCH 0/5] misc fixes

[nftables PATCH 0/5] misc fixes

[Eric Leblond]

Hello,

Here's a small patchset for nftables. It contains miscellaneous fixes
and improvements. I'm not sure for patch 2 ("expression") because it
may not be the best way to fix the crash. Patch 3 can also be discussed
because it suppress something that was done before in the code.

BR,
--
Eric

[nftables PATCH 1/5] doc: fix inversion of operator and object.

[Eric Leblond]

Signed-off-by: Eric Leblond <eric@regit.org>
---
 doc/nftables.xml |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/nftables.xml b/doc/nftables.xml
index ec6de38..055d4a6 100644
--- a/doc/nftables.xml
+++ b/doc/nftables.xml
@@ -150,13 +150,13 @@
 		<title>Tables</title>
 		<para>
 			<cmdsynopsis>
-				<command>table</command>
 				<group choice="req">
 					<arg>add</arg>
 					<arg>delete</arg>
 					<arg>list</arg>
 					<arg>flush</arg>
 				</group>
+				<command>table</command>
 				<arg choice="opt"><replaceable>family</replaceable></arg>
 				<arg choice="req"><replaceable>table</replaceable></arg>
 			</cmdsynopsis>
@@ -216,8 +216,8 @@
 		<title>Chains</title>
 		<para>
 			<cmdsynopsis>
-				<command>chain</command>
 				<arg choice="req">add</arg>
+				<command>chain</command>
 				<arg choice="opt"><replaceable>family</replaceable></arg>
 				<arg choice="req"><replaceable>table</replaceable></arg>
 				<arg choice="req"><replaceable>chain</replaceable></arg>
@@ -225,13 +225,13 @@
 				<arg choice="req"><replaceable>priority</replaceable></arg>
 			</cmdsynopsis>
 			<cmdsynopsis>
-				<command>chain</command>
 				<group choice="req">
 					<arg>add</arg>
 					<arg>delete</arg>
 					<arg>list</arg>
 					<arg>flush</arg>
 				</group>
+				<command>chain</command>
 				<arg choice="opt"><replaceable>family</replaceable></arg>
 				<arg choice="req"><replaceable>table</replaceable></arg>
 				<arg choice="req"><replaceable>chain</replaceable></arg>
@@ -287,11 +287,11 @@
 		<title>Rules</title>
 		<para>
 			<cmdsynopsis>
-				<command>rule</command>
 				<group choice="req">
 					<arg>add</arg>
 					<arg>delete</arg>
 				</group>
+				<command>rule</command>
 				<arg choice="opt"><replaceable>family</replaceable></arg>
 				<arg choice="req"><replaceable>table</replaceable></arg>
 				<arg choice="req"><replaceable>chain</replaceable></arg>
-- 
1.7.10.4

[nftables PATCH 2/5] expression: don't free TYPE_INVALID datatype

[Eric Leblond]

TYPE_INVALID datatype are unitialised and should not be free.

The following invalid command was segfaulting:
 nft add rule global filter  ip daddr . tcp dport { 192.168.0.1 . 22\; 192.168.0.3 . 89 } drop
with the following backtrace:
 (gdb) bt
 #0  0x00007ffff6f39295 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
 #1  0x00007ffff6f3c438 in __GI_abort () at abort.c:90
 #2  0x00007ffff6f7486b in __libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7ffff7070d28 "*** Error in `%s': %s: 0x%s ***\n") at ../sysdeps/unix/sysv/linux/libc_fatal.c:199
 #3  0x00007ffff6f7eac6 in malloc_printerr (action=3, str=0x7ffff706ccca "free(): invalid pointer", ptr=<optimized out>) at malloc.c:4902
 #4  0x00007ffff6f7f843 in _int_free (av=<optimized out>, p=0x428530, have_lock=0) at malloc.c:3758
 #5  0x000000000041aae8 in xfree (ptr=0x428540 <invalid_type>) at src/utils.c:29
 #6  0x000000000040bc43 in concat_type_destroy (dtype=0x428540 <invalid_type>) at src/datatype.c:690
 #7  0x000000000040cebf in concat_expr_destroy (expr=0x643b90) at src/expression.c:571
 #8  0x000000000040bef4 in expr_free (expr=0x643b90) at src/expression.c:67
 #9  0x000000000040cd8e in compound_expr_destroy (expr=0x643c20) at src/expression.c:542
 #10 0x000000000040bef4 in expr_free (expr=0x643c20) at src/expression.c:67
 #11 0x000000000041c314 in yydestruct (yymsg=0x4354a1 "Error: popping", yytype=242, yyvaluep=0x7fffffffbcf8, yylocationp=0x7fffffff9db8, scanner=0x643690, state=0x7fffffffdf90) at src/parser.y:398
 #12 0x000000000041ffb7 in nft_parse (scanner=0x643690, state=0x7fffffffdf90) at src/parser.c:5519
 #13 0x00000000004074df in nft_run (scanner=0x643690, state=0x7fffffffdf90, msgs=0x7fffffffdf80) at src/main.c:156
 #14 0x0000000000407a78 in main (argc=19, argv=0x7fffffffe698) at src/main.c:288

Signed-off-by: Eric Leblond <eric@regit.org>
---
 src/expression.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/expression.c b/src/expression.c
index 8cf3f62..e4108d0 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -568,7 +568,8 @@ void compound_expr_remove(struct expr *compound, struct expr *expr)
 
 static void concat_expr_destroy(struct expr *expr)
 {
-	concat_type_destroy(expr->dtype);
+	if (expr->dtype && expr->dtype->type != TYPE_INVALID)
+		concat_type_destroy(expr->dtype);
 	compound_expr_destroy(expr);
 }
 
-- 
1.7.10.4

[nftables PATCH 3/5] rule: list elements in set in any case

[Eric Leblond]

"nft list table" command was not displaying the elements of named
set. This was thus not possible to restore a ruleset by using the
listing output. This patch modifies the code to display the elements
of set in all cases.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 src/rule.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index e77323d..e7627a7 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -466,8 +466,7 @@ static int do_list_sets(struct netlink_ctx *ctx, const struct location *loc,
 		return -1;
 
 	list_for_each_entry_safe(set, nset, &ctx->list, list) {
-		if (set->flags & SET_F_ANONYMOUS &&
-		    netlink_get_setelems(ctx, &set->handle, loc, set) < 0)
+		if (netlink_get_setelems(ctx, &set->handle, loc, set) < 0)
 			return -1;
 		list_move_tail(&set->list, &table->sets);
 	}
-- 
1.7.10.4

[nftables PATCH 4/5] cli: add quit command

[Eric Leblond]

Signed-off-by: Eric Leblond <eric@regit.org>
---
 src/cli.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/cli.c b/src/cli.c
index a5a891e..541c26c 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -99,6 +99,11 @@ static void cli_complete(char *line)
 	if (*c == '\0')
 		return;
 
+	if (!strcmp(line, "quit")) {
+		cli_exit();
+		exit(0);
+	}
+
 	/* avoid duplicate history entries */
 	hist = history_get(history_length);
 	if (hist == NULL || strcmp(hist->line, line))
-- 
1.7.10.4

[nftables PATCH 5/5] cli: reset terminal when CTRL+d is pressed

[Eric Leblond]

The terminal was not correctly resetted when CTRL+d was pressed.
The result was that reset has to be called from shell after exit.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 src/cli.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/cli.c b/src/cli.c
index 541c26c..8875207 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -90,8 +90,11 @@ static void cli_complete(char *line)
 	LIST_HEAD(msgs);
 
 	line = cli_append_multiline(line);
-	if (line == NULL)
+	if (line == NULL) {
+		printf("\n");
+		cli_exit();
 		return;
+	}
 
 	for (c = line; *c != '\0'; c++)
 		if (!isspace(*c))
-- 
1.7.10.4

Re: [nftables PATCH 0/5] misc fixes

[Pablo Neira Ayuso]

Hi Eric,

On Sun, Jun 02, 2013 at 12:38:27PM +0200, Eric Leblond wrote:
> 
> Hello,
> 
> Here's a small patchset for nftables. It contains miscellaneous fixes
> and improvements. I'm not sure for patch 2 ("expression") because it
> may not be the best way to fix the crash.

I just sent you a patch to address this issue. I cannot find any
problem with yours though, but I think it's better if we don't make
any assumption and explicitly label dynamically allocated datatypes to
differenciate them of static ones.

> Patch 3 can also be discussed because it suppress something that was
> done before in the code.

Can you point to the patch that changed that line of code, I'm lacking
some context.

Regarding Patch 1, 4 and 5. Feel free to push them to the nftables
tree.

Thanks.

Re: [nftables PATCH 0/5] misc fixes

[Pablo Neira Ayuso]

On Thu, Jun 06, 2013 at 01:43:10PM +0200, Pablo Neira Ayuso wrote:
[...]
> > Patch 3 can also be discussed because it suppress something that was
> > done before in the code.
> 
> Can you point to the patch that changed that line of code, I'm lacking
> some context.

Oh, I think I get it. You mean that your patch is removing that line.
A quick tells me it's OK.

Regards.

Re: [nftables PATCH 0/5] misc fixes

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

Hi,

Le jeudi 06 juin 2013 à 14:02 +0200, Pablo Neira Ayuso a écrit :
> On Thu, Jun 06, 2013 at 01:43:10PM +0200, Pablo Neira Ayuso wrote:
> [...]
> > > Patch 3 can also be discussed because it suppress something that was
> > > done before in the code.
> > 
> > Can you point to the patch that changed that line of code, I'm lacking
> > some context.
> 
> Oh, I think I get it. You mean that your patch is removing that line.
> A quick tells me it's OK.

OK, I've just pushed the patches to nftables tree.

BR,
--
Eric

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]