[PATCH nft 1/2] src: define flowtable device compound as a list

[PATCH nft 1/2] src: define flowtable device compound as a list

[Pablo Neira Ayuso]

This fixes a memleak when releasing the compound expression via
expr_free().

Fixes: 92911b362e90 ("src: add support to add flowtables")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/parser_bison.y | 2 +-
 src/parser_json.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 1e2b30015f78..8ad581f69271 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1826,7 +1826,7 @@ flowtable_expr		:	'{'	flowtable_list_expr	'}'
 
 flowtable_list_expr	:	flowtable_expr_member
 			{
-				$$ = compound_expr_alloc(&@$, EXPR_INVALID);
+				$$ = compound_expr_alloc(&@$, EXPR_LIST);
 				compound_expr_add($$, $1);
 			}
 			|	flowtable_list_expr	COMMA	flowtable_expr_member
diff --git a/src/parser_json.c b/src/parser_json.c
index bc29dedf5b4c..55dbc177cc98 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -2914,7 +2914,7 @@ static struct cmd *json_parse_cmd_add_element(struct json_ctx *ctx,
 static struct expr *json_parse_flowtable_devs(struct json_ctx *ctx,
 					      json_t *root)
 {
-	struct expr *tmp, *expr = compound_expr_alloc(int_loc, EXPR_INVALID);
+	struct expr *tmp, *expr = compound_expr_alloc(int_loc, EXPR_LIST);
 	const char *dev;
 	json_t *value;
 	size_t index;
-- 
2.11.0

[PATCH nft 2/2,v2] flowtable: fix memleak in exit path

[Pablo Neira Ayuso]

From: Eric Jallot <ejallot@gmail.com>

Add missing loop in table_free().
Free all objects in flowtable_free() and add conditions in case of error recovery
in the parser (See commit 4be0a3f922a29).

Also, fix memleak in the parser.

This fixes the following memleak:

 # valgrind --leak-check=full nft add flowtable inet raw f '{ hook ingress priority filter; devices = { eth0 }; }'
 ==15414== Memcheck, a memory error detector
 ==15414== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
 ==15414== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
 ==15414== Command: nft add flowtable inet raw f {\ hook\ ingress\ priority\ filter;\ devices\ =\ {\ eth0\ };\ }
 ==15414==
 ==15414==
 ==15414== HEAP SUMMARY:
 ==15414==     in use at exit: 266 bytes in 4 blocks
 ==15414==   total heap usage: 55 allocs, 51 frees, 208,105 bytes allocated
 ==15414==
 ==15414== 5 bytes in 1 blocks are definitely lost in loss record 2 of 4
 ==15414==    at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
 ==15414==    by 0x5C64AA9: strdup (strdup.c:42)
 ==15414==    by 0x4E705ED: xstrdup (utils.c:75)
 ==15414==    by 0x4E93F01: nft_lex (scanner.l:648)
 ==15414==    by 0x4E85C1C: nft_parse (parser_bison.c:5577)
 ==15414==    by 0x4E75A07: nft_parse_bison_buffer (libnftables.c:375)
 ==15414==    by 0x4E75A07: nft_run_cmd_from_buffer (libnftables.c:443)
 ==15414==    by 0x40170F: main (main.c:326)
 ==15414==
 ==15414== 261 (128 direct, 133 indirect) bytes in 1 blocks are definitely lost in loss record 4 of 4
 ==15414==    at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
 ==15414==    by 0x4E705AD: xmalloc (utils.c:36)
 ==15414==    by 0x4E705AD: xzalloc (utils.c:65)
 ==15414==    by 0x4E560B6: expr_alloc (expression.c:45)
 ==15414==    by 0x4E56288: symbol_expr_alloc (expression.c:286)
 ==15414==    by 0x4E8A601: nft_parse (parser_bison.y:1842)
 ==15414==    by 0x4E75A07: nft_parse_bison_buffer (libnftables.c:375)
 ==15414==    by 0x4E75A07: nft_run_cmd_from_buffer (libnftables.c:443)
 ==15414==    by 0x40170F: main (main.c:326)

Fixes: 92911b362e906 ("src: add support to add flowtables")
Signed-off-by: Eric Jallot <ejallot@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: simplify original patch a bit.

 src/parser_bison.y |  1 +
 src/rule.c         | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 8ad581f69271..11f0dc8b2153 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1842,6 +1842,7 @@ flowtable_expr_member	:	STRING
 				$$ = symbol_expr_alloc(&@$, SYMBOL_VALUE,
 						       current_scope(state),
 						       $1);
+				xfree($1);
 			}
 			;
 
diff --git a/src/rule.c b/src/rule.c
index 2d35bae44c9e..d4d4add1afab 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1180,6 +1180,7 @@ struct table *table_alloc(void)
 void table_free(struct table *table)
 {
 	struct chain *chain, *next;
+	struct flowtable *ft, *nft;
 	struct set *set, *nset;
 	struct obj *obj, *nobj;
 
@@ -1189,6 +1190,8 @@ void table_free(struct table *table)
 		chain_free(chain);
 	list_for_each_entry_safe(set, nset, &table->sets, list)
 		set_free(set);
+	list_for_each_entry_safe(ft, nft, &table->flowtables, list)
+		flowtable_free(ft);
 	list_for_each_entry_safe(obj, nobj, &table->objs, list)
 		obj_free(obj);
 	handle_free(&table->handle);
@@ -2104,10 +2107,19 @@ struct flowtable *flowtable_get(struct flowtable *flowtable)
 
 void flowtable_free(struct flowtable *flowtable)
 {
+	int i;
+
 	if (--flowtable->refcnt > 0)
 		return;
 	handle_free(&flowtable->handle);
 	expr_free(flowtable->priority.expr);
+	expr_free(flowtable->dev_expr);
+
+	if (flowtable->dev_array != NULL) {
+		for (i = 0; i < flowtable->dev_array_len; i++)
+			xfree(flowtable->dev_array[i]);
+		xfree(flowtable->dev_array);
+	}
 	xfree(flowtable);
 }
 
-- 
2.11.0