[PATCH 0/4] bootconfig: Fix a parser bug

[PATCH 0/4] bootconfig: Fix a parser bug

[Masami Hiramatsu]

Hi,

Here are patches to fix 2 bugs in the parser. One issue happens
when a key has a siblings and the key repeated with brace after
sibling nodes. Another one is that the parser keeps tailing
spaces when we put a comment on the line.

For example, the minimum example of the 1st issue is here;

foo
bar
foo { buz }

This should be parsed as

foo.buz
bar

But the bootconfig parser parses it as foo.buz (no bar node)
because foo->bar link is unlinked when the brace ("foo {") was
found.

The second one is simpler, if we have

foo = val  # comment

The value's space after the word was not removed.

foo="val  "

But this also should be

foo="val"

If user needs tailing spaces, they can use quotes, e.g.

foo = "val  " # comment


Thank you,

---

Masami Hiramatsu (4):
      lib/bootconfig: Fix a bug of breaking existing tree nodes
      lib/bootconfig: Fix to remove tailing spaces after value
      tools/bootconfig: Add testcases for repeated key with brace
      tools/bootconfig: Add testcase for tailing space


 tools/bootconfig/test-bootconfig.sh |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[PATCH 1/4] lib/bootconfig: Fix a bug of breaking existing tree nodes

[Masami Hiramatsu]

Fix a bug of breaking existing tree nodes by parsing the second
and subsequent braces. Since the bootconfig parser uses the
node.next field as a flag of current parent node, but this will
breaks the existing tree if the same key node specified again
in the bootconfig.

For example, the following bootconfig should be foo.buz and bar.

foo
bar
foo { buz }

However, when parsing the brace "{", it breaks foo->bar link
by marking open-brace node. So the bootconfig unlinks bar
from the bootconfig internal tree.

This introduces a stack outside of the tree and record the
last open-brace on the stack instead of using node.next field.

Fixes: 76db5a27a827 ("bootconfig: Add Extra Boot Config support")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 0 files changed

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 2c905a91d4eb..b44bba0f1583 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -31,6 +31,8 @@ static size_t xbc_data_size __initdata;
 static struct xbc_node *last_parent __initdata;
 static const char *xbc_err_msg __initdata;
 static int xbc_err_pos __initdata;
+static int open_brace[XBC_DEPTH_MAX] __initdata;
+static int brace_index __initdata;
 
 static int __init xbc_parse_error(const char *msg, const char *p)
 {
@@ -431,27 +433,27 @@ static char *skip_spaces_until_newline(char *p)
 	return p;
 }
 
-static int __init __xbc_open_brace(void)
+static int __init __xbc_open_brace(char *p)
 {
-	/* Mark the last key as open brace */
-	last_parent->next = XBC_NODE_MAX;
+	/* Push the last key as open brace */
+	open_brace[brace_index++] = xbc_node_index(last_parent);
+	if (brace_index >= XBC_DEPTH_MAX)
+		return xbc_parse_error("Exceed max depth of braces", p);
 
 	return 0;
 }
 
 static int __init __xbc_close_brace(char *p)
 {
-	struct xbc_node *node;
-
-	if (!last_parent || last_parent->next != XBC_NODE_MAX)
+	brace_index--;
+	if (!last_parent || brace_index < 0 ||
+	    (open_brace[brace_index] != xbc_node_index(last_parent)))
 		return xbc_parse_error("Unexpected closing brace", p);
 
-	node = last_parent;
-	node->next = 0;
-	do {
-		node = xbc_node_get_parent(node);
-	} while (node && node->next != XBC_NODE_MAX);
-	last_parent = node;
+	if (brace_index == 0)
+		last_parent = NULL;
+	else
+		last_parent = &xbc_nodes[open_brace[brace_index - 1]];
 
 	return 0;
 }
@@ -661,7 +663,7 @@ static int __init xbc_open_brace(char **k, char *n)
 		return ret;
 	*k = n;
 
-	return __xbc_open_brace();
+	return __xbc_open_brace(n - 1);
 }
 
 static int __init xbc_close_brace(char **k, char *n)
@@ -681,6 +683,13 @@ static int __init xbc_verify_tree(void)
 	int i, depth, len, wlen;
 	struct xbc_node *n, *m;
 
+	/* Brace closing */
+	if (brace_index) {
+		n = &xbc_nodes[open_brace[brace_index]];
+		return xbc_parse_error("Brace is not closed",
+					xbc_node_get_data(n));
+	}
+
 	/* Empty tree */
 	if (xbc_node_num == 0) {
 		xbc_parse_error("Empty config", xbc_data);
@@ -745,6 +754,7 @@ void __init xbc_destroy_all(void)
 	xbc_node_num = 0;
 	memblock_free(__pa(xbc_nodes), sizeof(struct xbc_node) * XBC_NODE_MAX);
 	xbc_nodes = NULL;
+	brace_index = 0;
 }
 
 /**

[PATCH 2/4] lib/bootconfig: Fix to remove tailing spaces after value

[Masami Hiramatsu]

Fix to remove tailing spaces after value. If there is a space
after value, the bootconfig failed to remove it because it
applies strim() before replacing the delimiter with null.

For example,

foo = var    # comment

was parsed as below.

foo="var    "

but user will expect

foo="var"

This fixes it by applying strim() after removing the delimiter.

Fixes: 76db5a27a827 ("bootconfig: Add Extra Boot Config support")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 0 files changed

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index b44bba0f1583..649ed44f199c 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -494,8 +494,8 @@ static int __init __xbc_parse_value(char **__v, char **__n)
 			break;
 		}
 		if (strchr(",;\n#}", c)) {
-			v = strim(v);
 			*p++ = '\0';
+			v = strim(v);
 			break;
 		}
 	}

[PATCH 3/4] tools/bootconfig: Add testcases for repeated key with brace

[Masami Hiramatsu]

Add a testcase for repeated key with brace parsing issue.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/bootconfig/test-bootconfig.sh |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index 56284b98d8f0..95eec768b503 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -137,6 +137,18 @@ $BOOTCONF $INITRD > $TEMPCONF
 cat $TEMPCONF
 xpass grep \'\"string\"\' $TEMPCONF
 
+echo "Repeat same-key tree"
+cat > $TEMPCONF << EOF
+foo
+bar
+foo { buz }
+EOF
+echo > $INITRD
+
+xpass $BOOTCONF -a $TEMPCONF $INITRD
+$BOOTCONF $INITRD > $OUTFILE
+xpass grep -q "bar" $OUTFILE
+
 echo "=== expected failure cases ==="
 for i in samples/bad-* ; do
   xfail $BOOTCONF -a $i $INITRD

[PATCH 4/4] tools/bootconfig: Add testcase for tailing space

[Masami Hiramatsu]

Add testcases for removing/keeping tailing space
in the value.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/bootconfig/test-bootconfig.sh |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index 95eec768b503..d295e406a756 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -149,6 +149,19 @@ xpass $BOOTCONF -a $TEMPCONF $INITRD
 $BOOTCONF $INITRD > $OUTFILE
 xpass grep -q "bar" $OUTFILE
 
+
+echo "Remove/keep tailing spaces"
+cat > $TEMPCONF << EOF
+foo = val     # comment
+bar = "val2 " # comment
+EOF
+echo > $INITRD
+
+xpass $BOOTCONF -a $TEMPCONF $INITRD
+$BOOTCONF $INITRD > $OUTFILE
+xfail grep -q val[[:space:]] $OUTFILE
+xpass grep -q val2[[:space:]] $OUTFILE
+
 echo "=== expected failure cases ==="
 for i in samples/bad-* ; do
   xfail $BOOTCONF -a $i $INITRD

bootconfig: Fix parser bugs (Re: [PATCH 0/4] bootconfig: Fix a parser bug)

[Masami Hiramatsu]

Oops, the title is wrong, there are 2 bugs.

On Mon, 21 Sep 2020 18:44:33 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi,
> 
> Here are patches to fix 2 bugs in the parser. One issue happens
> when a key has a siblings and the key repeated with brace after
> sibling nodes. Another one is that the parser keeps tailing
> spaces when we put a comment on the line.
> 
> For example, the minimum example of the 1st issue is here;
> 
> foo
> bar
> foo { buz }
> 
> This should be parsed as
> 
> foo.buz
> bar
> 
> But the bootconfig parser parses it as foo.buz (no bar node)
> because foo->bar link is unlinked when the brace ("foo {") was
> found.
> 
> The second one is simpler, if we have
> 
> foo = val  # comment
> 
> The value's space after the word was not removed.
> 
> foo="val  "
> 
> But this also should be
> 
> foo="val"
> 
> If user needs tailing spaces, they can use quotes, e.g.
> 
> foo = "val  " # comment
> 
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (4):
>       lib/bootconfig: Fix a bug of breaking existing tree nodes
>       lib/bootconfig: Fix to remove tailing spaces after value
>       tools/bootconfig: Add testcases for repeated key with brace
>       tools/bootconfig: Add testcase for tailing space
> 
> 
>  tools/bootconfig/test-bootconfig.sh |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>


-- 
Masami Hiramatsu

Re: [PATCH 0/4] bootconfig: Fix a parser bug

[Masami Hiramatsu]

Hi Steve,

Thank you for merging previous 3 serieses!
Could you also pick this series as urgent-fix branch?

Thank you,

On Mon, 21 Sep 2020 18:44:33 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi,
> 
> Here are patches to fix 2 bugs in the parser. One issue happens
> when a key has a siblings and the key repeated with brace after
> sibling nodes. Another one is that the parser keeps tailing
> spaces when we put a comment on the line.
> 
> For example, the minimum example of the 1st issue is here;
> 
> foo
> bar
> foo { buz }
> 
> This should be parsed as
> 
> foo.buz
> bar
> 
> But the bootconfig parser parses it as foo.buz (no bar node)
> because foo->bar link is unlinked when the brace ("foo {") was
> found.
> 
> The second one is simpler, if we have
> 
> foo = val  # comment
> 
> The value's space after the word was not removed.
> 
> foo="val  "
> 
> But this also should be
> 
> foo="val"
> 
> If user needs tailing spaces, they can use quotes, e.g.
> 
> foo = "val  " # comment
> 
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (4):
>       lib/bootconfig: Fix a bug of breaking existing tree nodes
>       lib/bootconfig: Fix to remove tailing spaces after value
>       tools/bootconfig: Add testcases for repeated key with brace
>       tools/bootconfig: Add testcase for tailing space
> 
> 
>  tools/bootconfig/test-bootconfig.sh |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 0/4] bootconfig: Fix a parser bug

[Steven Rostedt]

On Wed, 23 Sep 2020 14:30:14 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi Steve,
> 
> Thank you for merging previous 3 serieses!
> Could you also pick this series as urgent-fix branch?
> 

Already pulled them and just finished testing them. Will push to Linus
in a bit.

-- Steve