Re: [PATCH v4 16/17] lib/bootconfig: check xbc_init_node() return in override path

[Google]

On Sat, 14 Mar 2026 23:01:54 +0000
Josh Law <objecting@xxxxxxxxxxxxx> wrote:

> The ':=' override path in xbc_parse_kv() calls xbc_init_node() to
> re-initialize an existing value node but does not check the return
> value.  If xbc_init_node() fails (data offset out of range), parsing
> silently continues with stale node data.
> 
> Add the missing error check to match the xbc_add_node() call path
> which already checks for failure.
> 
> In practice, a bootconfig using ':=' to override a value near the
> 32KB data limit could silently retain the old value, meaning a
> security-relevant boot parameter override (e.g., a trace filter or
> debug setting) would not take effect as intended.

OK, this is a real bug. It should be handled.

Fixes: e5efaeb8a8f5 ("bootconfig: Support mixing a value and subkeys under a key")

Thanks,

> 
> Signed-off-by: Josh Law <objecting@xxxxxxxxxxxxx>
> ---
>  lib/bootconfig.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index 038f56689a48..182d9d9bc5a6 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -728,7 +728,8 @@ static int __init xbc_parse_kv(char **k, char *v, int op)
>  		if (op == ':') {
>  			unsigned short nidx = child->next;
>  
> -			xbc_init_node(child, v, XBC_VALUE);
> +			if (xbc_init_node(child, v, XBC_VALUE) < 0)
> +				return xbc_parse_error("Failed to override value", v);
>  			child->next = nidx;	/* keep subkeys */
>  			goto array;
>  		}
> -- 
> 2.34.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>