Re: [PATCH v2 1/3] lib/bootconfig: fix off-by-one in xbc_verify_tree() unclosed brace error

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Thu, 12 Mar 2026 21:03:52 +0000
Josh Law <hlcj1234567@gmail.com> wrote:

> 12 Mar 2026 21:02:51 Steven Rostedt <rostedt@goodmis.org>:
> 
> > On Thu, 12 Mar 2026 19:11:41 +0000
> > Josh Law <hlcj1234567@gmail.com> wrote:
> >
> >> From: Josh Law <objecting@objecting.org>
> >>
> >> __xbc_open_brace() pushes entries with post-increment
> >> (open_brace[brace_index++]), so brace_index always points one past
> >> the last valid entry.  xbc_verify_tree() reads open_brace[brace_index]
> >> to report which brace is unclosed, but this is one past the last
> >> pushed entry and contains stale/zero data, causing the error message
> >> to reference the wrong node.
> >>
> >> Use open_brace[brace_index - 1] to correctly identify the unclosed
> >> brace.  brace_index is known to be > 0 here since we are inside the
> >> if (brace_index) guard.
> >>
> >> Signed-off-by: Josh Law <objecting@objecting.org>
> >
> > Nice catch. May I ask how you found this.
> >
> > Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Thanks for patch and review!

> >
> > -- Steve
> >
> >> ---
> >> lib/bootconfig.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> >> index 2bcd5c2aa87e..a1e6a2e14b01 100644
> >> --- a/lib/bootconfig.c
> >> +++ b/lib/bootconfig.c
> >> @@ -802,7 +802,7 @@ static int __init xbc_verify_tree(void)
> >>
> >>     /* Brace closing */
> >>     if (brace_index) {
> >> -       n = &xbc_nodes[open_brace[brace_index]];
> >> +       n = &xbc_nodes[open_brace[brace_index - 1]];
> >>         return xbc_parse_error("Brace is not closed",
> >>                     xbc_node_get_data(n));
> >>     }
> 
> Hi Steve,
> Thanks for the review!
> I found this while doing a manual audit of the bootconfig parser's error handling. I noticed that the post-increment in __xbc_open_brace() didn't seem to align with how xbc_verify_tree() was accessing the index. I verified it by intentionally passing a 
malformed config with an unclosed brace and saw it reporting a 'stale' or incorrect node location

Thanks, I confirmed it with below config.

$ cat samples/bad-non-closed-brace.bconf 
foo {
	bar {
		buz
}

This closed the 2nd `{`, but not close the first one.

Without patch;
$ ./bootconfig samples/bad-non-closed-brace.bconf 
Parse Error: Brace is not closed at 2:2

With this fix;
$ ./bootconfig samples/bad-non-closed-brace.bconf 
Parse Error: Brace is not closed at 1:1

Than you!

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