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

Josh Law posted 17 patches 3 weeks, 2 days ago
There is a newer version of this series
[PATCH v4 16/17] lib/bootconfig: check xbc_init_node() return in override path
Posted by Josh Law 3 weeks, 2 days ago
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.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 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
Re: [PATCH v4 16/17] lib/bootconfig: check xbc_init_node() return in override path
Posted by Masami Hiramatsu (Google) 3 weeks, 1 day ago
On Sat, 14 Mar 2026 23:01:54 +0000
Josh Law <objecting@objecting.org> 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@objecting.org>
> ---
>  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@kernel.org>