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

[Josh Law <hlcj1234567@gmail.com>]

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>
>
> -- 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