Re: [PATCH 1/3] tools/bootconfig: Close only opened braces in compact output

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] tools/bootconfig: stack overflow in xbc_show_compact_tree()
  on deep subkeys
--

commit de9746e7a830d520494d15bd2077bf3c3e2cb8d2
Author: Eduardo Mucelli Rezende Oliveira <edumucelli@gmail.com>

tools/bootconfig: Close only opened braces in compact output

This commit tracks the nodes for which opening braces were printed in
xbc_show_compact_tree() so that closing braces are only emitted for
matching nodes during backtracking.

> diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
> index 56d76bdeaa098..1bab0e685e2d8 100644
> --- a/tools/bootconfig/main.c
> +++ b/tools/bootconfig/main.c
> @@ -71,6 +71,7 @@ static int xbc_show_value(struct xbc_node *node, bool semicolon)
>  static void xbc_show_compact_tree(void)
>  {
>  	struct xbc_node *node, *cnode = NULL, *vnode;
> +	struct xbc_node *parent[XBC_DEPTH_MAX];
>  	int depth = 0, i;
>  
>  	node = xbc_root_node();
> @@ -100,7 +101,7 @@ static void xbc_show_compact_tree(void)
>  		}
>  		if (cnode && xbc_node_is_key(cnode)) {
>  			printf("%s {\n", xbc_node_get_data(node));
> -			depth++;
> +			parent[depth++] = node;

[Severity: Medium]
Can this write past the end of parent[] when a key with a value also has
subkeys nested deeper than XBC_DEPTH_MAX?

When a key has both a value and subkeys, the value node is stored as its
first child and the subkeys follow as next siblings of that value node.
However, xbc_verify_tree() only checks the first child for subkeys:

lib/bootconfig.c:xbc_verify_tree() {
    ...
		m = xbc_node_get_child(n);
		if (m && xbc_node_is_key(m)) {
			n = m;
			depth++;
			if (depth > XBC_DEPTH_MAX)
				return xbc_parse_error("Too many key words",
						xbc_node_get_data(n));
			continue;
		}
    ...
}

Because m is a value node rather than a key node, xbc_verify_tree() does
not descend into the subkeys under n and never enforces XBC_DEPTH_MAX on
that subtree.  When xbc_show_compact_tree() later walks those subkeys via
cnode = xbc_node_get_next(cnode), depth can exceed XBC_DEPTH_MAX and
overflow parent[] on the stack.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794333957.git.edumucelli@gmail.com?part=1