[PATCH v2 0/3] lib/bootconfig: three bug fixes

[PATCH v2 0/3] lib/bootconfig: three bug fixes

[Josh Law]

Three fixes for lib/bootconfig.c:

1. Fix off-by-one in xbc_verify_tree() unclosed brace error reporting.
2. Check bounds before writing in __xbc_open_brace() to prevent
   potential out-of-bounds writes.
3. Fix snprintf truncation check in xbc_node_compose_key_after().

Changes since v1:
- Added explicit From: lines at top of changelog.

Josh Law (3):
  lib/bootconfig: fix off-by-one in xbc_verify_tree() unclosed brace
    error
  lib/bootconfig: check bounds before writing in __xbc_open_brace()
  lib/bootconfig: fix snprintf truncation check in
    xbc_node_compose_key_after()

 lib/bootconfig.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.34.1

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

[Josh Law]

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>
---
 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));
 	}
-- 
2.34.1

[PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()

[Josh Law]

From: Josh Law <objecting@objecting.org>

The bounds check for brace_index happens after the array write.
While the current call pattern prevents an actual out-of-bounds
access (the previous call would have returned an error), the
write-before-check pattern is fragile and would become a real
out-of-bounds write if the error return were ever not propagated.

Move the bounds check before the array write so the function is
self-contained and safe regardless of caller behavior.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index a1e6a2e14b01..62b4ed7a0ba6 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -532,9 +532,9 @@ static char *skip_spaces_until_newline(char *p)
 static int __init __xbc_open_brace(char *p)
 {
 	/* 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);
+	open_brace[brace_index++] = xbc_node_index(last_parent);
 
 	return 0;
 }
-- 
2.34.1

[PATCH v2 3/3] lib/bootconfig: fix snprintf truncation check in xbc_node_compose_key_after()

[Josh Law]

From: Josh Law <objecting@objecting.org>

snprintf() returns the number of characters that would have been
written excluding the NUL terminator.  Output is truncated when the
return value is >= the buffer size, not just > the buffer size.

When ret == size, the current code takes the non-truncated path,
advancing buf by ret and reducing size to 0.  This is wrong because
the output was actually truncated (the last character was replaced by
NUL).  Fix by using >= so the truncation path is taken correctly.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 62b4ed7a0ba6..b0ef1e74e98a 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -316,7 +316,7 @@ int __init xbc_node_compose_key_after(struct xbc_node *root,
 			       depth ? "." : "");
 		if (ret < 0)
 			return ret;
-		if (ret > size) {
+		if (ret >= size) {
 			size = 0;
 		} else {
 			size -= ret;
-- 
2.34.1

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

[Steven Rostedt]

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));
>  	}

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

[Josh Law]

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

Re: [PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()

[Steven Rostedt]

On Thu, 12 Mar 2026 19:11:42 +0000
Josh Law <hlcj1234567@gmail.com> wrote:

> From: Josh Law <objecting@objecting.org>
> 
> The bounds check for brace_index happens after the array write.
> While the current call pattern prevents an actual out-of-bounds
> access (the previous call would have returned an error), the
> write-before-check pattern is fragile and would become a real
> out-of-bounds write if the error return were ever not propagated.
> 
> Move the bounds check before the array write so the function is
> self-contained and safe regardless of caller behavior.

This is the only place that increments the index, and the check is >=,
which means even if there was just one space left, it would fail.

As there's no other place that updates brace_index, I don't believe this
patch is needed. It could even replace the >= with ==.

-- Steve


> 
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  lib/bootconfig.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index a1e6a2e14b01..62b4ed7a0ba6 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -532,9 +532,9 @@ static char *skip_spaces_until_newline(char *p)
>  static int __init __xbc_open_brace(char *p)
>  {
>  	/* 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);
> +	open_brace[brace_index++] = xbc_node_index(last_parent);
>  
>  	return 0;
>  }

Re: [PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()

[Josh Law]

12 Mar 2026 21:06:31 Steven Rostedt <rostedt@goodmis.org>:

> On Thu, 12 Mar 2026 19:11:42 +0000
> Josh Law <hlcj1234567@gmail.com> wrote:
>
>> From: Josh Law <objecting@objecting.org>
>>
>> The bounds check for brace_index happens after the array write.
>> While the current call pattern prevents an actual out-of-bounds
>> access (the previous call would have returned an error), the
>> write-before-check pattern is fragile and would become a real
>> out-of-bounds write if the error return were ever not propagated.
>>
>> Move the bounds check before the array write so the function is
>> self-contained and safe regardless of caller behavior.
>
> This is the only place that increments the index, and the check is >=,
> which means even if there was just one space left, it would fail.
>
> As there's no other place that updates brace_index, I don't believe this
> patch is needed. It could even replace the >= with ==.
>
> -- Steve
>
>
>>
>> Signed-off-by: Josh Law <objecting@objecting.org>
>> ---
>> lib/bootconfig.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
>> index a1e6a2e14b01..62b4ed7a0ba6 100644
>> --- a/lib/bootconfig.c
>> +++ b/lib/bootconfig.c
>> @@ -532,9 +532,9 @@ static char *skip_spaces_until_newline(char *p)
>> static int __init __xbc_open_brace(char *p)
>> {
>>     /* 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);
>> +   open_brace[brace_index++] = xbc_node_index(last_parent);
>>
>>     return 0;
>> }

That's a fair point, Steve. Given that brace_index isn't touched elsewhere and the current check effectively prevents the overflow, I agree this isn't strictly necessary. I'll drop this patch and stick with the fix for the off-by-one reporting error instead. Thanks for the feedback!

Re: [PATCH v2 3/3] lib/bootconfig: fix snprintf truncation check in xbc_node_compose_key_after()

[Steven Rostedt]

On Thu, 12 Mar 2026 19:11:43 +0000
Josh Law <hlcj1234567@gmail.com> wrote:

> From: Josh Law <objecting@objecting.org>
> 
> snprintf() returns the number of characters that would have been
> written excluding the NUL terminator.  Output is truncated when the
> return value is >= the buffer size, not just > the buffer size.
> 
> When ret == size, the current code takes the non-truncated path,
> advancing buf by ret and reducing size to 0.  This is wrong because
> the output was actually truncated (the last character was replaced by
> NUL).  Fix by using >= so the truncation path is taken correctly.
> 
> Signed-off-by: Josh Law <objecting@objecting.org>

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 62b4ed7a0ba6..b0ef1e74e98a 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -316,7 +316,7 @@ int __init xbc_node_compose_key_after(struct xbc_node *root,
>  			       depth ? "." : "");
>  		if (ret < 0)
>  			return ret;
> -		if (ret > size) {
> +		if (ret >= size) {
>  			size = 0;
>  		} else {
>  			size -= ret;

Re: [PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()

[Josh Law]

12 Mar 2026 21:08:03 Josh Law <hlcj1234567@gmail.com>:

> 12 Mar 2026 21:06:31 Steven Rostedt <rostedt@goodmis.org>:
>
>> On Thu, 12 Mar 2026 19:11:42 +0000
>> Josh Law <hlcj1234567@gmail.com> wrote:
>>
>>> From: Josh Law <objecting@objecting.org>
>>>
>>> The bounds check for brace_index happens after the array write.
>>> While the current call pattern prevents an actual out-of-bounds
>>> access (the previous call would have returned an error), the
>>> write-before-check pattern is fragile and would become a real
>>> out-of-bounds write if the error return were ever not propagated.
>>>
>>> Move the bounds check before the array write so the function is
>>> self-contained and safe regardless of caller behavior.
>>
>> This is the only place that increments the index, and the check is >=,
>> which means even if there was just one space left, it would fail.
>>
>> As there's no other place that updates brace_index, I don't believe this
>> patch is needed. It could even replace the >= with ==.
>>
>> -- Steve
>>
>>
>>>
>>> Signed-off-by: Josh Law <objecting@objecting.org>
>>> ---
>>> lib/bootconfig.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
>>> index a1e6a2e14b01..62b4ed7a0ba6 100644
>>> --- a/lib/bootconfig.c
>>> +++ b/lib/bootconfig.c
>>> @@ -532,9 +532,9 @@ static char *skip_spaces_until_newline(char *p)
>>> static int __init __xbc_open_brace(char *p)
>>> {
>>>     /* 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);
>>> +   open_brace[brace_index++] = xbc_node_index(last_parent);
>>>
>>>     return 0;
>>> }
>
> That's a fair point, Steve. Given that brace_index isn't touched elsewhere and the current check effectively prevents the overflow, I agree this isn't strictly necessary. I'll drop this patch and stick with the fix for the off-by-one reporting error instead. Thanks for the feedback!

Wait Steve,
Thanks for the look. I see your point that it's currently redundant given the call patterns. It looks like Andrew has already merged this into the -mm tree, likely as a 'belt-and-suspenders' safety measure. I'll keep your feedback in mind for future cleanup, but I'm glad we got the other off-by-one fix in as well!

And in my opinion, merging it is a decent idea.

Re: [PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()

[Andrew Morton]

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

> > That's a fair point, Steve. Given that brace_index isn't touched elsewhere and the current check effectively prevents the overflow, I agree this isn't strictly necessary. I'll drop this patch and stick with the fix for the off-by-one reporting error instead. Thanks for the feedback!
> 
> Wait Steve,
> Thanks for the look. I see your point that it's currently redundant given the call patterns. It looks like Andrew has already merged this into the -mm tree, likely as a 'belt-and-suspenders' safety measure. I'll keep your feedback in mind for future cleanup, but I'm glad we got the other off-by-one fix in as well!

Please wordwrap the emails.

> And in my opinion, merging it is a decent idea.

You've changed your position without explaining why?

Re: [PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()

[Josh Law]

12 Mar 2026 21:28:11 Andrew Morton <akpm@linux-foundation.org>:

> On Thu, 12 Mar 2026 21:09:52 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
>
>>> That's a fair point, Steve. Given that brace_index isn't touched elsewhere and the current check effectively prevents the overflow, I agree this isn't strictly necessary. I'll drop this patch and stick with the fix for the off-by-one reporting error instead. Thanks for the feedback!
>>
>> Wait Steve,
>> Thanks for the look. I see your point that it's currently redundant given the call patterns. It looks like Andrew has already merged this into the -mm tree, likely as a 'belt-and-suspenders' safety measure. I'll keep your feedback in mind for future cleanup, but I'm glad we got the other off-by-one fix in as well!
>
> Please wordwrap the emails.
>
>> And in my opinion, merging it is a decent idea.
>
> You've changed your position without explaining why?

Sorry, I think it should be merged because it's better to be safe than sorry, I know there is different methods of implementation, but this one still works... I know it's churn (and I'm sorry)

Re: [PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()

[Steven Rostedt]

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

> 12 Mar 2026 21:28:11 Andrew Morton <akpm@linux-foundation.org>:
> 
> > On Thu, 12 Mar 2026 21:09:52 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
> >  
> >>> That's a fair point, Steve. Given that brace_index isn't touched
> >>> elsewhere and the current check effectively prevents the overflow, I
> >>> agree this isn't strictly necessary. I'll drop this patch and stick
> >>> with the fix for the off-by-one reporting error instead. Thanks for
> >>> the feedback!  
> >>
> >> Wait Steve,
> >> Thanks for the look. I see your point that it's currently redundant
> >> given the call patterns. It looks like Andrew has already merged this
> >> into the -mm tree, likely as a 'belt-and-suspenders' safety measure.
> >> I'll keep your feedback in mind for future cleanup, but I'm glad we
> >> got the other off-by-one fix in as well!  
> >
> > Please wordwrap the emails.
> >  
> >> And in my opinion, merging it is a decent idea.  
> >
> > You've changed your position without explaining why?  
> 

/me wraps your email (claws-mail does that nicely ;-)

> Sorry, I think it should be merged because it's better to be safe than
> sorry, I know there is different methods of implementation, but this one
> still works... I know it's churn (and I'm sorry)

It's not the only place that does that. And since the value is safe as is,
I rather not touch it.

-- Steve

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

[Masami Hiramatsu]

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>

Re: [PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()

[Masami Hiramatsu]

On Thu, 12 Mar 2026 19:11:42 +0000
Josh Law <hlcj1234567@gmail.com> wrote:

> From: Josh Law <objecting@objecting.org>
> 
> The bounds check for brace_index happens after the array write.
> While the current call pattern prevents an actual out-of-bounds
> access (the previous call would have returned an error), the
> write-before-check pattern is fragile and would become a real
> out-of-bounds write if the error return were ever not propagated.
> 
> Move the bounds check before the array write so the function is
> self-contained and safe regardless of caller behavior.
> 

Hmm good catch. This is a kind of specification mistake.
If we pass below to the bootconfig tool, currently it fails.
----
key1 {
key2 {
key3 {
key4 {
key5 {
key6 {
key7 {
key8 {
key9 {
key10 {
key11 {
key12 {
key13 {
key14 {
key15 {
key16 {
}}}}}}}}}}}}}}}}
----
But since "open_brace[]" array has 16 entries, it should
accept the 16th brace.

Let me add a good and a bad example for this case too.

Thanks,

> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  lib/bootconfig.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index a1e6a2e14b01..62b4ed7a0ba6 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -532,9 +532,9 @@ static char *skip_spaces_until_newline(char *p)
>  static int __init __xbc_open_brace(char *p)
>  {
>  	/* 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);
> +	open_brace[brace_index++] = xbc_node_index(last_parent);
>  
>  	return 0;
>  }
> -- 
> 2.34.1
> 


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

Re: [PATCH v2 3/3] lib/bootconfig: fix snprintf truncation check in xbc_node_compose_key_after()

[Masami Hiramatsu]

On Thu, 12 Mar 2026 19:11:43 +0000
Josh Law <hlcj1234567@gmail.com> wrote:

> From: Josh Law <objecting@objecting.org>
> 
> snprintf() returns the number of characters that would have been
> written excluding the NUL terminator.  Output is truncated when the
> return value is >= the buffer size, not just > the buffer size.
> 
> When ret == size, the current code takes the non-truncated path,
> advancing buf by ret and reducing size to 0.  This is wrong because
> the output was actually truncated (the last character was replaced by
> NUL).  Fix by using >= so the truncation path is taken correctly.
> 

OK, but this one is a minor issue, because either way, remaining
size becomes 0.

		ret = snprintf(buf, size, "%s%s", xbc_node_get_data(node),
			       depth ? "." : "");
		if (ret < 0)
			return ret;
		if (ret > size) {
			size = 0;
		} else {
			size -= ret; // if ret == size, the size becomes 0
			buf += ret;
		}

Anyway, to be clear the correct error case handling, this
should be applied.

Thank you!

> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  lib/bootconfig.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index 62b4ed7a0ba6..b0ef1e74e98a 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -316,7 +316,7 @@ int __init xbc_node_compose_key_after(struct xbc_node *root,
>  			       depth ? "." : "");
>  		if (ret < 0)
>  			return ret;
> -		if (ret > size) {
> +		if (ret >= size) {
>  			size = 0;
>  		} else {
>  			size -= ret;
> -- 
> 2.34.1
> 


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

Re: [PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()

[Masami Hiramatsu]

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

> 12 Mar 2026 21:28:11 Andrew Morton <akpm@linux-foundation.org>:
> 
> > On Thu, 12 Mar 2026 21:09:52 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
> >
> >>> That's a fair point, Steve. Given that brace_index isn't touched elsewhere and the current check effectively prevents the overflow, I agree this isn't strictly necessary. I'll drop this patch and stick with the fix for the off-by-one reporting error instead. Thanks for the feedback!
> >>
> >> Wait Steve,
> >> Thanks for the look. I see your point that it's currently redundant given the call patterns. It looks like Andrew has already merged this into the -mm tree, likely as a 'belt-and-suspenders' safety measure. I'll keep your feedback in mind for future cleanup, but I'm glad we got the other off-by-one fix in as well!
> >
> > Please wordwrap the emails.
> >
> >> And in my opinion, merging it is a decent idea.
> >
> > You've changed your position without explaining why?
> 
> Sorry, I think it should be merged because it's better to be safe than sorry, I know there is different methods of implementation, but this one still works... I know it's churn (and I'm sorry)

I would like to keep this original >= because it is safer.
Andrew, I will pick these patches with my test patch.

Thank you,


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

[PATCH] bootconfig: Add bootconfig tests about braces

[Masami Hiramatsu (Google)]

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

Add more bootconfig tests for checking the error message of
non closing brace and max number of nested braces.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 .../bootconfig/samples/bad-non-closed-brace.bconf  |    4 ++++
 tools/bootconfig/samples/bad-over-max-brace.bconf  |   19 +++++++++++++++++++
 .../bootconfig/samples/exp-good-nested-brace.bconf |    1 +
 tools/bootconfig/samples/good-nested-brace.bconf   |   18 ++++++++++++++++++
 tools/bootconfig/test-bootconfig.sh                |    9 +++++++++
 5 files changed, 51 insertions(+)
 create mode 100644 tools/bootconfig/samples/bad-non-closed-brace.bconf
 create mode 100644 tools/bootconfig/samples/bad-over-max-brace.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-nested-brace.bconf
 create mode 100644 tools/bootconfig/samples/good-nested-brace.bconf

diff --git a/tools/bootconfig/samples/bad-non-closed-brace.bconf b/tools/bootconfig/samples/bad-non-closed-brace.bconf
new file mode 100644
index 000000000000..6ed9f3363dde
--- /dev/null
+++ b/tools/bootconfig/samples/bad-non-closed-brace.bconf
@@ -0,0 +1,4 @@
+foo {
+ bar {
+   buz
+ }
diff --git a/tools/bootconfig/samples/bad-over-max-brace.bconf b/tools/bootconfig/samples/bad-over-max-brace.bconf
new file mode 100644
index 000000000000..74b5dc9e21dc
--- /dev/null
+++ b/tools/bootconfig/samples/bad-over-max-brace.bconf
@@ -0,0 +1,19 @@
+key1 {
+key2 {
+key3 {
+key4 {
+key5 {
+key6 {
+key7 {
+key8 {
+key9 {
+key10 {
+key11 {
+key12 {
+key13 {
+key14 {
+key15 {
+key16 {
+key17 {
+}}}}}}}}}}}}}}}}}
+
diff --git a/tools/bootconfig/samples/exp-good-nested-brace.bconf b/tools/bootconfig/samples/exp-good-nested-brace.bconf
new file mode 100644
index 000000000000..19e0f51b4553
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-nested-brace.bconf
@@ -0,0 +1 @@
+key1.key2.key3.key4.key5.key6.key7.key8.key9.key10.key11.key12.key13.key14.key15.key16;
diff --git a/tools/bootconfig/samples/good-nested-brace.bconf b/tools/bootconfig/samples/good-nested-brace.bconf
new file mode 100644
index 000000000000..980d094f296e
--- /dev/null
+++ b/tools/bootconfig/samples/good-nested-brace.bconf
@@ -0,0 +1,18 @@
+key1 {
+key2 {
+key3 {
+key4 {
+key5 {
+key6 {
+key7 {
+key8 {
+key9 {
+key10 {
+key11 {
+key12 {
+key13 {
+key14 {
+key15 {
+key16 {
+}}}}}}}}}}}}}}}}
+
diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index be9bd18b1d56..fc69f815ce4a 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -171,6 +171,15 @@ $BOOTCONF $INITRD > $OUTFILE
 xfail grep -q 'val[[:space:]]' $OUTFILE
 xpass grep -q 'val2[[:space:]]' $OUTFILE
 
+echo "Showing correct line:column of no closing brace"
+cat > $TEMPCONF << EOF
+foo {
+bar {
+}
+EOF
+$BOOTCONF -a $TEMPCONF $INITRD 2> $OUTFILE
+xpass grep -q "1:1" $OUTFILE
+
 echo "=== expected failure cases ==="
 for i in samples/bad-* ; do
   xfail $BOOTCONF -a $i $INITRD

Re: [PATCH] bootconfig: Add bootconfig tests about braces

[Josh Law]

13 Mar 2026 04:19:06 Masami Hiramatsu (Google) <mhiramat@kernel.org>:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Add more bootconfig tests for checking the error message of
> non closing brace and max number of nested braces.
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> .../bootconfig/samples/bad-non-closed-brace.bconf  |    4 ++++
> tools/bootconfig/samples/bad-over-max-brace.bconf  |   19 +++++++++++++++++++
> .../bootconfig/samples/exp-good-nested-brace.bconf |    1 +
> tools/bootconfig/samples/good-nested-brace.bconf   |   18 ++++++++++++++++++
> tools/bootconfig/test-bootconfig.sh                |    9 +++++++++
> 5 files changed, 51 insertions(+)
> create mode 100644 tools/bootconfig/samples/bad-non-closed-brace.bconf
> create mode 100644 tools/bootconfig/samples/bad-over-max-brace.bconf
> create mode 100644 tools/bootconfig/samples/exp-good-nested-brace.bconf
> create mode 100644 tools/bootconfig/samples/good-nested-brace.bconf
>
> diff --git a/tools/bootconfig/samples/bad-non-closed-brace.bconf b/tools/bootconfig/samples/bad-non-closed-brace.bconf
> new file mode 100644
> index 000000000000..6ed9f3363dde
> --- /dev/null
> +++ b/tools/bootconfig/samples/bad-non-closed-brace.bconf
> @@ -0,0 +1,4 @@
> +foo {
> + bar {
> +   buz
> + }
> diff --git a/tools/bootconfig/samples/bad-over-max-brace.bconf b/tools/bootconfig/samples/bad-over-max-brace.bconf
> new file mode 100644
> index 000000000000..74b5dc9e21dc
> --- /dev/null
> +++ b/tools/bootconfig/samples/bad-over-max-brace.bconf
> @@ -0,0 +1,19 @@
> +key1 {
> +key2 {
> +key3 {
> +key4 {
> +key5 {
> +key6 {
> +key7 {
> +key8 {
> +key9 {
> +key10 {
> +key11 {
> +key12 {
> +key13 {
> +key14 {
> +key15 {
> +key16 {
> +key17 {
> +}}}}}}}}}}}}}}}}}
> +
> diff --git a/tools/bootconfig/samples/exp-good-nested-brace.bconf b/tools/bootconfig/samples/exp-good-nested-brace.bconf
> new file mode 100644
> index 000000000000..19e0f51b4553
> --- /dev/null
> +++ b/tools/bootconfig/samples/exp-good-nested-brace.bconf
> @@ -0,0 +1 @@
> +key1.key2.key3.key4.key5.key6.key7.key8.key9.key10.key11.key12.key13.key14.key15.key16;
> diff --git a/tools/bootconfig/samples/good-nested-brace.bconf b/tools/bootconfig/samples/good-nested-brace.bconf
> new file mode 100644
> index 000000000000..980d094f296e
> --- /dev/null
> +++ b/tools/bootconfig/samples/good-nested-brace.bconf
> @@ -0,0 +1,18 @@
> +key1 {
> +key2 {
> +key3 {
> +key4 {
> +key5 {
> +key6 {
> +key7 {
> +key8 {
> +key9 {
> +key10 {
> +key11 {
> +key12 {
> +key13 {
> +key14 {
> +key15 {
> +key16 {
> +}}}}}}}}}}}}}}}}
> +
> diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
> index be9bd18b1d56..fc69f815ce4a 100755
> --- a/tools/bootconfig/test-bootconfig.sh
> +++ b/tools/bootconfig/test-bootconfig.sh
> @@ -171,6 +171,15 @@ $BOOTCONF $INITRD > $OUTFILE
> xfail grep -q 'val[[:space:]]' $OUTFILE
> xpass grep -q 'val2[[:space:]]' $OUTFILE
>
> +echo "Showing correct line:column of no closing brace"
> +cat > $TEMPCONF << EOF
> +foo {
> +bar {
> +}
> +EOF
> +$BOOTCONF -a $TEMPCONF $INITRD 2> $OUTFILE
> +xpass grep -q "1:1" $OUTFILE
> +
> echo "=== expected failure cases ==="
> for i in samples/bad-* ; do
>    xfail $BOOTCONF -a $i $INITRD

Acked-By: Josh Law <objecting@objecting.org>