12 Mar 2026 21:06:31 Steven Rostedt <[email protected]>: > On Thu, 12 Mar 2026 19:11:42 +0000 > Josh Law <[email protected]> wrote: > >> From: Josh Law <[email protected]> >> >> 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 <[email protected]> >> --- >> 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!
