[PATCH nft 0/4] glob and maximum number of includes

[PATCH nft 0/4] glob and maximum number of includes

[Pablo Neira Ayuso]

Hi Laurent,

This approach maintains an array of stacks per depth.

The initial three patches comes as a preparation. The last patch is
aiming to fix the issue with glob and the maximum number of includes.

Thanks for your detailed feedback and explanations.

Pablo Neira Ayuso (4):
  scanner: call scanner_push_file() after scanner_push_file()
  scanner: add indesc_file_alloc() helper function
  scanner: call scanner_push_indesc() after scanner_push_file()
  scanner: multi-level input file stack for glob

 include/list.h     |  30 ++++++++++++++
 include/parser.h   |   3 +-
 src/parser_bison.y |   5 ++-
 src/scanner.l      | 120 +++++++++++++++++++++++++++++++++++++----------------
 4 files changed, 119 insertions(+), 39 deletions(-)

-- 
2.11.0

[PATCH nft 1/4] scanner: call scanner_push_file() after scanner_push_file()

[Pablo Neira Ayuso]

Update include_file() to return FILE *.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/scanner.l | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/src/scanner.l b/src/scanner.l
index ecf2354e3c2f..fe70df5c18ec 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -714,8 +714,8 @@ static void scanner_push_file(struct nft_ctx *nft, void *scanner,
 	scanner_push_indesc(state, indesc);
 }
 
-static int include_file(struct nft_ctx *nft, void *scanner,
-			const char *filename, const struct location *loc)
+static FILE *include_file(struct nft_ctx *nft, void *scanner,
+			  const char *filename, const struct location *loc)
 {
 	struct parser_state *state = yyget_extra(scanner);
 	struct error_record *erec;
@@ -733,11 +733,11 @@ static int include_file(struct nft_ctx *nft, void *scanner,
 			     filename, strerror(errno));
 		goto err;
 	}
-	scanner_push_file(nft, scanner, f, filename, loc);
-	return 0;
+
+	return f;
 err:
 	erec_queue(erec, state->msgs);
-	return -1;
+	return NULL;
 }
 
 static int include_glob(struct nft_ctx *nft, void *scanner, const char *pattern,
@@ -749,6 +749,7 @@ static int include_glob(struct nft_ctx *nft, void *scanner, const char *pattern,
 	glob_t glob_data;
 	unsigned int i;
 	int flags = 0;
+	FILE *f;
 	int ret;
 	char *p;
 
@@ -804,9 +805,11 @@ static int include_glob(struct nft_ctx *nft, void *scanner, const char *pattern,
 			if (len == 0 || path[len - 1] == '/')
 				continue;
 
-			ret = include_file(nft, scanner, path, loc);
-			if (ret != 0)
+			f = include_file(nft, scanner, path, loc);
+			if (!f)
 				goto err;
+
+			scanner_push_file(nft, scanner, f, path, loc);
 		}
 
 		globfree(&glob_data);
@@ -841,7 +844,14 @@ err:
 int scanner_read_file(struct nft_ctx *nft, const char *filename,
 		      const struct location *loc)
 {
-	return include_file(nft, nft->scanner, filename, loc);
+	FILE *f;
+
+	f = include_file(nft, nft->scanner, filename, loc);
+	if (!f)
+		return -1;
+
+	scanner_push_file(nft, nft->scanner, f, filename, loc);
+	return 0;
 }
 
 static bool search_in_include_path(const char *filename)
-- 
2.11.0

[PATCH nft 2/4] scanner: add indesc_file_alloc() helper function

[Pablo Neira Ayuso]

New helper function to allocate the file input_descriptor.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/scanner.l | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/scanner.l b/src/scanner.l
index fe70df5c18ec..56f6e9956791 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -665,6 +665,22 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 
 %%
 
+static struct input_descriptor *
+indesc_file_alloc(FILE *f, const char *filename, const struct location *loc)
+{
+	struct input_descriptor *indesc;
+
+	indesc = xzalloc(sizeof(struct input_descriptor));
+	if (loc != NULL)
+		indesc->location = *loc;
+	indesc->type	= INDESC_FILE;
+	indesc->name	= xstrdup(filename);
+	indesc->f	= f;
+	init_pos(indesc);
+
+	return indesc;
+}
+
 static void scanner_push_indesc(struct parser_state *state,
 				struct input_descriptor *indesc)
 {
@@ -702,15 +718,7 @@ static void scanner_push_file(struct nft_ctx *nft, void *scanner,
 	b = yy_create_buffer(f, YY_BUF_SIZE, scanner);
 	yypush_buffer_state(b, scanner);
 
-	indesc = xzalloc(sizeof(struct input_descriptor));
-
-	if (loc != NULL)
-		indesc->location = *loc;
-	indesc->type	= INDESC_FILE;
-	indesc->name	= xstrdup(filename);
-	indesc->f	= f;
-	init_pos(indesc);
-
+	indesc = indesc_file_alloc(f, filename, loc);
 	scanner_push_indesc(state, indesc);
 }
 
-- 
2.11.0

[PATCH nft 3/4] scanner: call scanner_push_indesc() after scanner_push_file()

[Pablo Neira Ayuso]

Just a preparation patch.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/scanner.l | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/scanner.l b/src/scanner.l
index 56f6e9956791..9584f61c489c 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -707,19 +707,16 @@ static void scanner_pop_buffer(yyscan_t scanner)
 	scanner_pop_indesc(state);
 }
 
-static void scanner_push_file(struct nft_ctx *nft, void *scanner,
-			      FILE *f, const char *filename,
-			      const struct location *loc)
+static struct input_descriptor *
+scanner_push_file(struct nft_ctx *nft, void *scanner, FILE *f,
+		  const char *filename, const struct location *loc)
 {
-	struct parser_state *state = yyget_extra(scanner);
-	struct input_descriptor *indesc;
 	YY_BUFFER_STATE b;
 
 	b = yy_create_buffer(f, YY_BUF_SIZE, scanner);
 	yypush_buffer_state(b, scanner);
 
-	indesc = indesc_file_alloc(f, filename, loc);
-	scanner_push_indesc(state, indesc);
+	return indesc_file_alloc(f, filename, loc);
 }
 
 static FILE *include_file(struct nft_ctx *nft, void *scanner,
@@ -805,6 +802,7 @@ static int include_glob(struct nft_ctx *nft, void *scanner, const char *pattern,
 
 		/* reverse alphabetical order due to stack */
 		for (i = glob_data.gl_pathc; i > 0; i--) {
+			struct input_descriptor *indesc;
 
 			path = glob_data.gl_pathv[i-1];
 
@@ -817,7 +815,8 @@ static int include_glob(struct nft_ctx *nft, void *scanner, const char *pattern,
 			if (!f)
 				goto err;
 
-			scanner_push_file(nft, scanner, f, path, loc);
+			indesc = scanner_push_file(nft, scanner, f, path, loc);
+			scanner_push_indesc(state, indesc);
 		}
 
 		globfree(&glob_data);
@@ -852,13 +851,17 @@ err:
 int scanner_read_file(struct nft_ctx *nft, const char *filename,
 		      const struct location *loc)
 {
+	struct parser_state *state = yyget_extra(nft->scanner);
+	struct input_descriptor *indesc;
 	FILE *f;
 
 	f = include_file(nft, nft->scanner, filename, loc);
 	if (!f)
 		return -1;
 
-	scanner_push_file(nft, nft->scanner, f, filename, loc);
+	indesc = scanner_push_file(nft, nft->scanner, f, filename, loc);
+	scanner_push_indesc(state, indesc);
+
 	return 0;
 }
 
-- 
2.11.0

[PATCH nft 4/4] scanner: multi-level input file stack for glob

[Pablo Neira Ayuso]

This patch updates indesc_list to become an array of stacks. Each stack
represents the files that have been included at this depth.

The scanner_add_indesc() function adds the indesc to this depth, this is
called in case the user specifies wildcards.

Otherwise, the scanner_push_indesc() function for regular inclusion.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/list.h     | 30 +++++++++++++++++++++++++++++
 include/parser.h   |  3 +--
 src/parser_bison.y |  5 ++++-
 src/scanner.l      | 55 ++++++++++++++++++++++++++++++++++++++++--------------
 4 files changed, 76 insertions(+), 17 deletions(-)

diff --git a/include/list.h b/include/list.h
index 75d292124010..29f5ca739632 100644
--- a/include/list.h
+++ b/include/list.h
@@ -22,6 +22,17 @@ struct list_head {
 	struct list_head *next, *prev;
 };
 
+/**
+ * list_is_first -- tests whether @list is the first entry in list @head
+ * @list: the entry to test
+ * @head: the head of the list
+ */
+static inline int list_is_first(const struct list_head *list,
+				const struct list_head *head)
+{
+	return list->prev == head;
+}
+
 #define LIST_HEAD_INIT(name) { &(name), &(name) }
 
 #define LIST_HEAD(name) \
@@ -623,3 +634,22 @@ static inline void hlist_add_after(struct hlist_node *n,
 	     pos = n)
 
 #endif
+
+/**
+ * list_prev_entry - get the prev element in list
+ * @pos:        the type * to cursor
+ * @member:     the name of the list_head within the struct.
+ */
+#define list_prev_entry(pos, member) \
+	list_entry((pos)->member.prev, typeof(*(pos)), member)
+
+/**
+ * list_last_entry - get the last element from a list
+ * @ptr:        the list head to take the element from.
+ * @type:       the type of the struct this is embedded in.
+ * @member:     the name of the list_head within the struct.
+ *
+ * Note, that list is expected to be not empty.
+ */
+#define list_last_entry(ptr, type, member) \
+	list_entry((ptr)->prev, type, member)
diff --git a/include/parser.h b/include/parser.h
index 949284d9466c..d24133059203 100644
--- a/include/parser.h
+++ b/include/parser.h
@@ -15,9 +15,8 @@
 
 struct parser_state {
 	struct input_descriptor		*indesc;
-	struct input_descriptor		*indescs[MAX_INCLUDE_DEPTH];
 	unsigned int			indesc_idx;
-	struct list_head		indesc_list;
+	struct list_head		indesc_list[MAX_INCLUDE_DEPTH];
 
 	struct list_head		*msgs;
 	unsigned int			nerrs;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index f5d7026a8574..216126472687 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -45,11 +45,14 @@ void parser_init(struct nft_ctx *nft, struct parser_state *state,
 		 struct list_head *msgs, struct list_head *cmds,
 		 struct scope *top_scope)
 {
+	int i;
+
 	memset(state, 0, sizeof(*state));
 	state->msgs = msgs;
 	state->cmds = cmds;
 	state->scopes[0] = scope_init(top_scope, NULL);
-	init_list_head(&state->indesc_list);
+	for (i = 0; i < MAX_INCLUDE_DEPTH; i++)
+		init_list_head(&state->indesc_list[i]);
 }
 
 static void yyerror(struct location *loc, struct nft_ctx *nft, void *scanner,
diff --git a/src/scanner.l b/src/scanner.l
index 9584f61c489c..9a6734dd4ec4 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -681,22 +681,44 @@ indesc_file_alloc(FILE *f, const char *filename, const struct location *loc)
 	return indesc;
 }
 
+static inline struct list_head *indesc_list(struct parser_state *state)
+{
+	return &state->indesc_list[state->indesc_idx];
+}
+
 static void scanner_push_indesc(struct parser_state *state,
 				struct input_descriptor *indesc)
 {
-	state->indescs[state->indesc_idx] = indesc;
-	state->indesc = state->indescs[state->indesc_idx++];
-	list_add_tail(&indesc->list, &state->indesc_list);
+	state->indesc = indesc;
+	list_add_tail(&indesc->list, indesc_list(state));
+	state->indesc_idx++;
+}
+
+static void scanner_add_indesc(struct parser_state *state,
+			       struct input_descriptor *indesc)
+{
+	state->indesc = indesc;
+	list_add_tail(&indesc->list, indesc_list(state));
 }
 
 static void scanner_pop_indesc(struct parser_state *state)
 {
-	state->indesc_idx--;
+	if (list_empty(indesc_list(state)))
+		state->indesc_idx--;
 
-	if (state->indesc_idx > 0)
-		state->indesc = state->indescs[state->indesc_idx - 1];
-	else
+	if (state->indesc_idx == 0) {
 		state->indesc = NULL;
+		return;
+	}
+
+	if (list_is_first(&state->indesc->list, indesc_list(state))) {
+		state->indesc_idx--;
+		state->indesc =
+			list_last_entry(indesc_list(state),
+					struct input_descriptor, list);
+	} else {
+		state->indesc = list_prev_entry(state->indesc, list);
+	}
 }
 
 static void scanner_pop_buffer(yyscan_t scanner)
@@ -816,8 +838,10 @@ static int include_glob(struct nft_ctx *nft, void *scanner, const char *pattern,
 				goto err;
 
 			indesc = scanner_push_file(nft, scanner, f, path, loc);
-			scanner_push_indesc(state, indesc);
+			scanner_add_indesc(state, indesc);
 		}
+		if (glob_data.gl_pathc)
+			state->indesc_idx++;
 
 		globfree(&glob_data);
 
@@ -962,14 +986,17 @@ static void input_descriptor_destroy(const struct input_descriptor *indesc)
 static void input_descriptor_list_destroy(struct parser_state *state)
 {
 	struct input_descriptor *indesc, *next;
+	int i;
 
-	list_for_each_entry_safe(indesc, next, &state->indesc_list, list) {
-		if (indesc->f) {
-			fclose(indesc->f);
-			indesc->f = NULL;
+	for (i = 0; i < MAX_INCLUDE_DEPTH; i++) {
+		list_for_each_entry_safe(indesc, next, &state->indesc_list[i], list) {
+			if (indesc->f) {
+				fclose(indesc->f);
+				indesc->f = NULL;
+			}
+			list_del(&indesc->list);
+			input_descriptor_destroy(indesc);
 		}
-		list_del(&indesc->list);
-		input_descriptor_destroy(indesc);
 	}
 }
 
-- 
2.11.0

Re: [PATCH nft 0/4] glob and maximum number of includes

[Pablo Neira Ayuso]

On Tue, Feb 11, 2020 at 09:23:04PM +0100, Pablo Neira Ayuso wrote:
> Hi Laurent,
> 
> This approach maintains an array of stacks per depth.
> 
> The initial three patches comes as a preparation. The last patch is
> aiming to fix the issue with glob and the maximum number of includes.

Hm, unfortunately, my patchset does not work. I'm going to toss this
and go back to your approach.