misc fixes

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

---
 doc/nftables.xml |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

index ec6de38..055d4a6 100644
@@ -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

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

---
 src/expression.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 8cf3f62..e4108d0 100644
@@ -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

"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.

---
 src/rule.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index e77323d..e7627a7 100644
@@ -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

---
 src/cli.c |    5 +++++
 1 file changed, 5 insertions(+)

index a5a891e..541c26c 100644
@@ -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

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

---
 src/cli.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

index 541c26c..8875207 100644
@@ -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

Hi Eric,

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.

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.

On Thu, Jun 06, 2013 at 01:43:10PM +0200, Pablo Neira Ayuso wrote:
[...]

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

Regards.

Hi,

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

BR,
--
Eric