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@xxxxxxxxxxx>:

> 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.
>
> 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@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;
>> }

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!