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

[Google]

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

> From: Josh Law <objecting@xxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>