[PATCH v4 00/17] bootconfig: fixes, cleanups, and modernization

[PATCH v4 00/17] bootconfig: fixes, cleanups, and modernization

[Josh Law]

This series addresses a collection of issues found during a review of
lib/bootconfig.c, include/linux/bootconfig.h, and tools/bootconfig,
ranging from off-by-one errors and unchecked return values to coding
style and API modernization.

Changes since v3:
  - Added commit descriptions to all patches that were missing them
    (patches 2, 3, 4, 7).
  - Added real-world impact statements to all bug-fix patches
    (patches 8, 9, 15, 16).

Changes since v2:
  - Added "validate child node index in xbc_verify_tree()" —
    xbc_verify_tree() validated next-node indices but not child indices;
    an out-of-bounds child would cause xbc_node_get_child() to access
    memory beyond the xbc_nodes array (patch 15).
  - Added "check xbc_init_node() return in override path" — the ':='
    override path in xbc_parse_kv() ignored xbc_init_node()'s return
    value, silently continuing with stale node data on failure
    (patch 16).
  - Added "fix fd leak in load_xbc_file() on fstat failure" — if
    fstat() failed after open() succeeded, the file descriptor was
    leaked (patch 17).

Changes since v1:
  - Dropped "return empty string instead of NULL from
    xbc_node_get_data()" — returning "" causes false matches in
    xbc_node_match_prefix() because strncmp(..., "", 0) always
    returns 0.

Bug fixes:
  - Fix off-by-one in xbc_verify_tree() where a next-node index equal
    to xbc_node_num passes the bounds check despite being out of range;
    a malformed bootconfig could cause an out-of-bounds read of kernel
    memory during tree traversal at boot time (patch 8).
  - Move xbc_node_num increment to after xbc_init_node() validation
    so a failed init does not leave a partially initialized node
    counted in the array; on a maximum-size bootconfig, the
    uninitialized node could be traversed leading to unpredictable
    boot behavior (patch 9).
  - Validate child node indices in xbc_verify_tree() alongside the
    existing next-node check; without this, a corrupt bootconfig could
    trigger an out-of-bounds memory access via an invalid child index
    during tree traversal (patch 15).
  - Check xbc_init_node() return value in the ':=' override path; a
    bootconfig using ':=' near the 32KB data limit could silently
    retain the old value, meaning a security-relevant boot parameter
    override would not take effect (patch 16).
  - Fix file descriptor leak in tools/bootconfig load_xbc_file()
    when fstat() fails (patch 17).

Correctness:
  - Add missing __init annotations to skip_comment() and
    skip_spaces_until_newline() so their memory can be reclaimed
    after init (patch 1).
  - Narrow the flag parameter in node creation helpers from uint32_t
    to uint16_t to match the xbc_node.data field width (patch 6).
  - Constify the xbc_calc_checksum() data parameter since it only
    reads the buffer (patch 12).

Cleanups:
  - Fix comment typos (patches 2-3), missing blank line before
    kerneldoc (patch 4), inconsistent if/else bracing (patches 5, 7).
  - Drop redundant memset after memblock_alloc which already returns
    zeroed memory; switch the userspace path from malloc to calloc
    to match (patch 10).

Modernization:
  - Replace open-coded __attribute__((__packed__)) with the __packed
    macro, adding the definition to the tools/bootconfig shim header
    (patches 11, 14).
  - Replace the catch-all linux/kernel.h include with the specific
    headers needed: linux/cache.h, linux/compiler.h, and
    linux/sprintf.h (patch 13).

Build-tested with both the in-kernel build (lib/bootconfig.o,
init/main.o) and the userspace tools/bootconfig build. All 70
tools/bootconfig test cases pass.

Josh Law (17):
  lib/bootconfig: add missing __init annotations to static helpers
  lib/bootconfig: fix typo "initiized" in xbc_root_node() kerneldoc
  lib/bootconfig: fix typo "uder" in xbc_node_find_next_leaf()
  lib/bootconfig: add blank line before xbc_get_info() kerneldoc
  lib/bootconfig: fix inconsistent if/else bracing
  lib/bootconfig: narrow flag parameter type from uint32_t to uint16_t
  lib/bootconfig: fix inconsistent if/else bracing in __xbc_add_key()
  lib/bootconfig: fix off-by-one in xbc_verify_tree() next node check
  lib/bootconfig: increment xbc_node_num after node init succeeds
  lib/bootconfig: drop redundant memset of xbc_nodes
  bootconfig: use __packed macro for struct xbc_node
  bootconfig: constify xbc_calc_checksum() data parameter
  lib/bootconfig: replace linux/kernel.h with specific includes
  bootconfig: add __packed definition to tools/bootconfig shim header
  lib/bootconfig: validate child node index in xbc_verify_tree()
  lib/bootconfig: check xbc_init_node() return in override path
  tools/bootconfig: fix fd leak in load_xbc_file() on fstat failure

 include/linux/bootconfig.h                  |  6 +--
 lib/bootconfig.c                            | 54 ++++++++++++---------
 tools/bootconfig/include/linux/bootconfig.h |  1 +
 tools/bootconfig/main.c                     |  4 +-
 4 files changed, 39 insertions(+), 26 deletions(-)

-- 
2.34.1

[PATCH v4 01/17] lib/bootconfig: add missing __init annotations to static helpers

[Josh Law]

skip_comment() and skip_spaces_until_newline() are static functions
called exclusively from __init code paths but lack the __init
annotation themselves. Add it so their memory can be reclaimed after
init.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index b0ef1e74e98a..51fd2299ec0f 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -509,7 +509,7 @@ static inline __init bool xbc_valid_keyword(char *key)
 	return *key == '\0';
 }
 
-static char *skip_comment(char *p)
+static char __init *skip_comment(char *p)
 {
 	char *ret;
 
@@ -522,7 +522,7 @@ static char *skip_comment(char *p)
 	return ret;
 }
 
-static char *skip_spaces_until_newline(char *p)
+static char __init *skip_spaces_until_newline(char *p)
 {
 	while (isspace(*p) && *p != '\n')
 		p++;
-- 
2.34.1

[PATCH v4 02/17] lib/bootconfig: fix typo "initiized" in xbc_root_node() kerneldoc

[Josh Law]

Fix "initiized" to "initialized" in the xbc_root_node() kerneldoc
comment.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 51fd2299ec0f..53aedc042f6e 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -112,7 +112,7 @@ static int __init xbc_parse_error(const char *msg, const char *p)
  * xbc_root_node() - Get the root node of extended boot config
  *
  * Return the address of root node of extended boot config. If the
- * extended boot config is not initiized, return NULL.
+ * extended boot config is not initialized, return NULL.
  */
 struct xbc_node * __init xbc_root_node(void)
 {
-- 
2.34.1

[PATCH v4 03/17] lib/bootconfig: fix typo "uder" in xbc_node_find_next_leaf()

[Josh Law]

Fix "uder" to "under" in the xbc_node_find_next_leaf() kerneldoc
comment.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 53aedc042f6e..35091617bca5 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -364,7 +364,7 @@ struct xbc_node * __init xbc_node_find_next_leaf(struct xbc_node *root,
 			node = xbc_node_get_parent(node);
 			if (node == root)
 				return NULL;
-			/* User passed a node which is not uder parent */
+			/* User passed a node which is not under parent */
 			if (WARN_ON(!node))
 				return NULL;
 		}
-- 
2.34.1

[PATCH v4 04/17] lib/bootconfig: add blank line before xbc_get_info() kerneldoc

[Josh Law]

Add the missing blank line between xbc_free_mem() and the
xbc_get_info() kerneldoc block so that documentation parsers do not
associate the comment with the wrong function.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 35091617bca5..e955d2f7e7ca 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -79,6 +79,7 @@ static inline void xbc_free_mem(void *addr, size_t size, bool early)
 	free(addr);
 }
 #endif
+
 /**
  * xbc_get_info() - Get the information of loaded boot config
  * @node_size: A pointer to store the number of nodes.
-- 
2.34.1

[PATCH v4 05/17] lib/bootconfig: fix inconsistent if/else bracing

[Josh Law]

When one branch of a conditional uses braces, both branches should
use them per kernel coding style.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index e955d2f7e7ca..45db51bc9cc7 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -473,8 +473,9 @@ static struct xbc_node * __init __xbc_add_sibling(char *data, uint32_t flag, boo
 				sib->next = xbc_node_index(node);
 			}
 		}
-	} else
+	} else {
 		xbc_parse_error("Too many nodes", data);
+	}
 
 	return node;
 }
@@ -992,8 +993,9 @@ int __init xbc_init(const char *data, size_t size, const char **emsg, int *epos)
 		if (emsg)
 			*emsg = xbc_err_msg;
 		_xbc_exit(true);
-	} else
+	} else {
 		ret = xbc_node_num;
+	}
 
 	return ret;
 }
-- 
2.34.1

[PATCH v4 06/17] lib/bootconfig: narrow flag parameter type from uint32_t to uint16_t

[Josh Law]

The flag parameter in the node creation helpers only ever carries
XBC_KEY (0) or XBC_VALUE (0x8000), both of which fit in uint16_t.
Using uint16_t matches the width of xbc_node.data where the flag is
ultimately stored.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 45db51bc9cc7..34bdc2d13881 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -408,7 +408,7 @@ const char * __init xbc_node_find_next_key_value(struct xbc_node *root,
 
 /* XBC parse and tree build */
 
-static int __init xbc_init_node(struct xbc_node *node, char *data, uint32_t flag)
+static int __init xbc_init_node(struct xbc_node *node, char *data, uint16_t flag)
 {
 	unsigned long offset = data - xbc_data;
 
@@ -422,7 +422,7 @@ static int __init xbc_init_node(struct xbc_node *node, char *data, uint32_t flag
 	return 0;
 }
 
-static struct xbc_node * __init xbc_add_node(char *data, uint32_t flag)
+static struct xbc_node * __init xbc_add_node(char *data, uint16_t flag)
 {
 	struct xbc_node *node;
 
@@ -452,7 +452,7 @@ static inline __init struct xbc_node *xbc_last_child(struct xbc_node *node)
 	return node;
 }
 
-static struct xbc_node * __init __xbc_add_sibling(char *data, uint32_t flag, bool head)
+static struct xbc_node * __init __xbc_add_sibling(char *data, uint16_t flag, bool head)
 {
 	struct xbc_node *sib, *node = xbc_add_node(data, flag);
 
@@ -480,17 +480,17 @@ static struct xbc_node * __init __xbc_add_sibling(char *data, uint32_t flag, boo
 	return node;
 }
 
-static inline struct xbc_node * __init xbc_add_sibling(char *data, uint32_t flag)
+static inline struct xbc_node * __init xbc_add_sibling(char *data, uint16_t flag)
 {
 	return __xbc_add_sibling(data, flag, false);
 }
 
-static inline struct xbc_node * __init xbc_add_head_sibling(char *data, uint32_t flag)
+static inline struct xbc_node * __init xbc_add_head_sibling(char *data, uint16_t flag)
 {
 	return __xbc_add_sibling(data, flag, true);
 }
 
-static inline __init struct xbc_node *xbc_add_child(char *data, uint32_t flag)
+static inline __init struct xbc_node *xbc_add_child(char *data, uint16_t flag)
 {
 	struct xbc_node *node = xbc_add_sibling(data, flag);
 
-- 
2.34.1

[PATCH v4 07/17] lib/bootconfig: fix inconsistent if/else bracing in __xbc_add_key()

[Josh Law]

When one branch of a conditional uses braces, both branches should
use them per coding-style section 3.1.  Add the missing braces to
the if/else blocks in __xbc_add_key().

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 34bdc2d13881..58d6ae297280 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -657,9 +657,9 @@ static int __init __xbc_add_key(char *k)
 	if (unlikely(xbc_node_num == 0))
 		goto add_node;
 
-	if (!last_parent)	/* the first level */
+	if (!last_parent) {	/* the first level */
 		node = find_match_node(xbc_nodes, k);
-	else {
+	} else {
 		child = xbc_node_get_child(last_parent);
 		/* Since the value node is the first child, skip it. */
 		if (child && xbc_node_is_value(child))
@@ -667,9 +667,9 @@ static int __init __xbc_add_key(char *k)
 		node = find_match_node(child, k);
 	}
 
-	if (node)
+	if (node) {
 		last_parent = node;
-	else {
+	} else {
 add_node:
 		node = xbc_add_child(k, XBC_KEY);
 		if (!node)
-- 
2.34.1

[PATCH v4 08/17] lib/bootconfig: fix off-by-one in xbc_verify_tree() next node check

[Josh Law]

Valid node indices are 0 to xbc_node_num-1, so a next value equal to
xbc_node_num is out of bounds.  Use >= instead of > to catch this.

A malformed or corrupt bootconfig could pass tree verification with
an out-of-bounds next index.  On subsequent tree traversal at boot
time, xbc_node_get_next() would return a pointer past the allocated
xbc_nodes array, causing an out-of-bounds read of kernel memory.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 58d6ae297280..56fbedc9e725 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -816,7 +816,7 @@ static int __init xbc_verify_tree(void)
 	}
 
 	for (i = 0; i < xbc_node_num; i++) {
-		if (xbc_nodes[i].next > xbc_node_num) {
+		if (xbc_nodes[i].next >= xbc_node_num) {
 			return xbc_parse_error("No closing brace",
 				xbc_node_get_data(xbc_nodes + i));
 		}
-- 
2.34.1

[PATCH v4 09/17] lib/bootconfig: increment xbc_node_num after node init succeeds

[Josh Law]

Move the xbc_node_num increment to after xbc_init_node() so a failed
init does not leave a partially initialized node counted in the array.

If xbc_init_node() fails on a data offset at the boundary of a
maximum-size bootconfig, the pre-incremented count causes subsequent
tree verification and traversal to consider the uninitialized node as
valid, potentially leading to an out-of-bounds read or unpredictable
boot behavior.

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 56fbedc9e725..06e8a79ab472 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -429,9 +429,10 @@ static struct xbc_node * __init xbc_add_node(char *data, uint16_t flag)
 	if (xbc_node_num == XBC_NODE_MAX)
 		return NULL;
 
-	node = &xbc_nodes[xbc_node_num++];
+	node = &xbc_nodes[xbc_node_num];
 	if (xbc_init_node(node, data, flag) < 0)
 		return NULL;
+	xbc_node_num++;
 
 	return node;
 }
-- 
2.34.1

[PATCH v4 10/17] lib/bootconfig: drop redundant memset of xbc_nodes

[Josh Law]

memblock_alloc() already returns zeroed memory, so the explicit memset
in xbc_init() is redundant. Switch the userspace xbc_alloc_mem() from
malloc() to calloc() so both paths return zeroed memory, and remove
the separate memset call.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 06e8a79ab472..fe1053043752 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -71,7 +71,7 @@ static inline void __init xbc_free_mem(void *addr, size_t size, bool early)
 
 static inline void *xbc_alloc_mem(size_t size)
 {
-	return malloc(size);
+	return calloc(1, size);
 }
 
 static inline void xbc_free_mem(void *addr, size_t size, bool early)
@@ -982,7 +982,6 @@ int __init xbc_init(const char *data, size_t size, const char **emsg, int *epos)
 		_xbc_exit(true);
 		return -ENOMEM;
 	}
-	memset(xbc_nodes, 0, sizeof(struct xbc_node) * XBC_NODE_MAX);
 
 	ret = xbc_parse_tree();
 	if (!ret)
-- 
2.34.1

[PATCH v4 11/17] bootconfig: use __packed macro for struct xbc_node

[Josh Law]

Replace the open-coded __attribute__((__packed__)) with the kernel
__packed macro for consistency with the rest of the kernel.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 include/linux/bootconfig.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index 25df9260d206..c37e0096c4f1 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -53,7 +53,7 @@ struct xbc_node {
 	uint16_t child;
 	uint16_t parent;
 	uint16_t data;
-} __attribute__ ((__packed__));
+} __packed;
 
 #define XBC_KEY		0
 #define XBC_VALUE	(1 << 15)
-- 
2.34.1

[PATCH v4 12/17] bootconfig: constify xbc_calc_checksum() data parameter

[Josh Law]

xbc_calc_checksum() only reads the data buffer, so mark the parameter
as const void * and the internal pointer as const unsigned char *.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 include/linux/bootconfig.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index c37e0096c4f1..d78c2b62debf 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -36,9 +36,9 @@ bool __init cmdline_has_extra_options(void);
  * The checksum will be used with the BOOTCONFIG_MAGIC and the size for
  * embedding the bootconfig in the initrd image.
  */
-static inline __init uint32_t xbc_calc_checksum(void *data, uint32_t size)
+static inline __init uint32_t xbc_calc_checksum(const void *data, uint32_t size)
 {
-	unsigned char *p = data;
+	const unsigned char *p = data;
 	uint32_t ret = 0;
 
 	while (size--)
-- 
2.34.1

[PATCH v4 13/17] lib/bootconfig: replace linux/kernel.h with specific includes

[Josh Law]

linux/kernel.h is a legacy catch-all header. Replace it with the
specific headers actually needed: linux/cache.h for SMP_CACHE_BYTES,
linux/compiler.h for unlikely(), and linux/sprintf.h for snprintf().

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index fe1053043752..0823491221f4 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -17,7 +17,9 @@
 #include <linux/bug.h>
 #include <linux/ctype.h>
 #include <linux/errno.h>
-#include <linux/kernel.h>
+#include <linux/cache.h>
+#include <linux/compiler.h>
+#include <linux/sprintf.h>
 #include <linux/memblock.h>
 #include <linux/string.h>
 
-- 
2.34.1

[PATCH v4 14/17] bootconfig: add __packed definition to tools/bootconfig shim header

[Josh Law]

The tools/bootconfig userspace build includes the main bootconfig.h
via a shim header that defines kernel macros for userspace. Add the
__packed macro so the struct xbc_node declaration works after the
conversion from open-coded __attribute__((__packed__)).

Signed-off-by: Josh Law <objecting@objecting.org>
---
 tools/bootconfig/include/linux/bootconfig.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/bootconfig/include/linux/bootconfig.h b/tools/bootconfig/include/linux/bootconfig.h
index 6784296a0692..41c50ab95ba5 100644
--- a/tools/bootconfig/include/linux/bootconfig.h
+++ b/tools/bootconfig/include/linux/bootconfig.h
@@ -48,6 +48,7 @@ static inline char *strim(char *s)
 
 #define __init
 #define __initdata
+#define __packed	__attribute__((__packed__))
 
 #include "../../../../include/linux/bootconfig.h"
 
-- 
2.34.1

[PATCH v4 15/17] lib/bootconfig: validate child node index in xbc_verify_tree()

[Josh Law]

xbc_verify_tree() validates that each node's next index is within
bounds, but does not check the child index.  Add the same bounds
check for the child field.

Without this check, a corrupt bootconfig that passes next-index
validation could still trigger an out-of-bounds memory access via an
invalid child index when xbc_node_get_child() is called during tree
traversal at boot time.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/bootconfig.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 0823491221f4..038f56689a48 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -823,6 +823,10 @@ static int __init xbc_verify_tree(void)
 			return xbc_parse_error("No closing brace",
 				xbc_node_get_data(xbc_nodes + i));
 		}
+		if (xbc_nodes[i].child >= xbc_node_num) {
+			return xbc_parse_error("Broken child node",
+				xbc_node_get_data(xbc_nodes + i));
+		}
 	}
 
 	/* Key tree limitation check */
-- 
2.34.1

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

[Josh Law]

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

[PATCH v4 17/17] tools/bootconfig: fix fd leak in load_xbc_file() on fstat failure

[Josh Law]

If fstat() fails after open() succeeds, load_xbc_file() returns
-errno without closing the file descriptor.  Add the missing close()
call on the error path.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 tools/bootconfig/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index 55d59ed507d5..8078fee0b75b 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -162,8 +162,10 @@ static int load_xbc_file(const char *path, char **buf)
 	if (fd < 0)
 		return -errno;
 	ret = fstat(fd, &stat);
-	if (ret < 0)
+	if (ret < 0) {
+		close(fd);
 		return -errno;
+	}
 
 	ret = load_xbc_fd(fd, buf, stat.st_size);
 
-- 
2.34.1

Re: [PATCH v4 09/17] lib/bootconfig: increment xbc_node_num after node init succeeds

[Masami Hiramatsu]

On Sat, 14 Mar 2026 23:01:47 +0000
Josh Law <objecting@objecting.org> wrote:

> Move the xbc_node_num increment to after xbc_init_node() so a failed
> init does not leave a partially initialized node counted in the array.
> 
> If xbc_init_node() fails on a data offset at the boundary of a
> maximum-size bootconfig, the pre-incremented count causes subsequent
> tree verification and traversal to consider the uninitialized node as
> valid, potentially leading to an out-of-bounds read or unpredictable
> boot behavior.

In that case, it returns a parse error(-ENOMEM) and the parsing stops.
This seems a hardening not a fix unless actual example you can show.

Thank you,

> 
> 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 56fbedc9e725..06e8a79ab472 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -429,9 +429,10 @@ static struct xbc_node * __init xbc_add_node(char *data, uint16_t flag)
>  	if (xbc_node_num == XBC_NODE_MAX)
>  		return NULL;
>  
> -	node = &xbc_nodes[xbc_node_num++];
> +	node = &xbc_nodes[xbc_node_num];
>  	if (xbc_init_node(node, data, flag) < 0)
>  		return NULL;
> +	xbc_node_num++;
>  
>  	return node;
>  }
> -- 
> 2.34.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v4 17/17] tools/bootconfig: fix fd leak in load_xbc_file() on fstat failure

[Masami Hiramatsu]

On Sat, 14 Mar 2026 23:01:55 +0000
Josh Law <objecting@objecting.org> wrote:

> If fstat() fails after open() succeeds, load_xbc_file() returns
> -errno without closing the file descriptor.  Add the missing close()
> call on the error path.
> 

OK, but anyway this exits soon after failure. So it does not
cause actual problem.

Fixes: 950313ebf79c ("tools: bootconfig: Add bootconfig command")

> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  tools/bootconfig/main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
> index 55d59ed507d5..8078fee0b75b 100644
> --- a/tools/bootconfig/main.c
> +++ b/tools/bootconfig/main.c
> @@ -162,8 +162,10 @@ static int load_xbc_file(const char *path, char **buf)
>  	if (fd < 0)
>  		return -errno;
>  	ret = fstat(fd, &stat);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		close(fd);
>  		return -errno;
> +	}
>  
>  	ret = load_xbc_fd(fd, buf, stat.st_size);
>  
> -- 
> 2.34.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v4 02/17] lib/bootconfig: fix typo "initiized" in xbc_root_node() kerneldoc

[Masami Hiramatsu]

Hi,

Thanks for cleaning ups. Can you fold [2/17][3/17][4/17] into 1 patch?
Those are just typo fixes and index/space fix, IOW, cosmetic change.

Thanks,

On Sat, 14 Mar 2026 23:01:40 +0000
Josh Law <objecting@objecting.org> wrote:

> Fix "initiized" to "initialized" in the xbc_root_node() kerneldoc
> comment.
> 
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  lib/bootconfig.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index 51fd2299ec0f..53aedc042f6e 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -112,7 +112,7 @@ static int __init xbc_parse_error(const char *msg, const char *p)
>   * xbc_root_node() - Get the root node of extended boot config
>   *
>   * Return the address of root node of extended boot config. If the
> - * extended boot config is not initiized, return NULL.
> + * extended boot config is not initialized, return NULL.
>   */
>  struct xbc_node * __init xbc_root_node(void)
>  {
> -- 
> 2.34.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v4 14/17] bootconfig: add __packed definition to tools/bootconfig shim header

[Masami Hiramatsu]

On Sat, 14 Mar 2026 23:01:52 +0000
Josh Law <objecting@objecting.org> wrote:

> The tools/bootconfig userspace build includes the main bootconfig.h
> via a shim header that defines kernel macros for userspace. Add the
> __packed macro so the struct xbc_node declaration works after the
> conversion from open-coded __attribute__((__packed__)).
> 

NACK. Please do not self recover after break something by yourself
in the same series.

> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  tools/bootconfig/include/linux/bootconfig.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/bootconfig/include/linux/bootconfig.h b/tools/bootconfig/include/linux/bootconfig.h
> index 6784296a0692..41c50ab95ba5 100644
> --- a/tools/bootconfig/include/linux/bootconfig.h
> +++ b/tools/bootconfig/include/linux/bootconfig.h
> @@ -48,6 +48,7 @@ static inline char *strim(char *s)
>  
>  #define __init
>  #define __initdata
> +#define __packed	__attribute__((__packed__))
>  
>  #include "../../../../include/linux/bootconfig.h"
>  
> -- 
> 2.34.1
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v4 11/17] bootconfig: use __packed macro for struct xbc_node

[Masami Hiramatsu]

On Sat, 14 Mar 2026 23:01:49 +0000
Josh Law <objecting@objecting.org> wrote:

> Replace the open-coded __attribute__((__packed__)) with the kernel
> __packed macro for consistency with the rest of the kernel.

NACK. Since I made this header to be shared with user tool.
As you sent in [14/17], this just breaks tools/bootconfig.

Thanks,

> 
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  include/linux/bootconfig.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
> index 25df9260d206..c37e0096c4f1 100644
> --- a/include/linux/bootconfig.h
> +++ b/include/linux/bootconfig.h
> @@ -53,7 +53,7 @@ struct xbc_node {
>  	uint16_t child;
>  	uint16_t parent;
>  	uint16_t data;
> -} __attribute__ ((__packed__));
> +} __packed;
>  
>  #define XBC_KEY		0
>  #define XBC_VALUE	(1 << 15)
> -- 
> 2.34.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v4 08/17] lib/bootconfig: fix off-by-one in xbc_verify_tree() next node check

[Masami Hiramatsu]

On Sat, 14 Mar 2026 23:01:46 +0000
Josh Law <objecting@objecting.org> wrote:

> Valid node indices are 0 to xbc_node_num-1, so a next value equal to
> xbc_node_num is out of bounds.  Use >= instead of > to catch this.
> 
> A malformed or corrupt bootconfig could pass tree verification with
> an out-of-bounds next index.  On subsequent tree traversal at boot
> time, xbc_node_get_next() would return a pointer past the allocated
> xbc_nodes array, causing an out-of-bounds read of kernel memory.
> 

Thanks, but How? Do you have any actual config example?
Unless that, I would like to treat this as a minor fix.

Thanks,

> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  lib/bootconfig.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index 58d6ae297280..56fbedc9e725 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -816,7 +816,7 @@ static int __init xbc_verify_tree(void)
>  	}
>  
>  	for (i = 0; i < xbc_node_num; i++) {
> -		if (xbc_nodes[i].next > xbc_node_num) {
> +		if (xbc_nodes[i].next >= xbc_node_num) {
>  			return xbc_parse_error("No closing brace",
>  				xbc_node_get_data(xbc_nodes + i));
>  		}
> -- 
> 2.34.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v4 07/17] lib/bootconfig: fix inconsistent if/else bracing in __xbc_add_key()

[Masami Hiramatsu]

On Sat, 14 Mar 2026 23:01:45 +0000
Josh Law <objecting@objecting.org> wrote:

> When one branch of a conditional uses braces, both branches should
> use them per coding-style section 3.1.  Add the missing braces to
> the if/else blocks in __xbc_add_key().

It is just a cosmetic cleanup.
Can you fold this with other typo fixes?

THank you,

> 
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  lib/bootconfig.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index 34bdc2d13881..58d6ae297280 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -657,9 +657,9 @@ static int __init __xbc_add_key(char *k)
>  	if (unlikely(xbc_node_num == 0))
>  		goto add_node;
>  
> -	if (!last_parent)	/* the first level */
> +	if (!last_parent) {	/* the first level */
>  		node = find_match_node(xbc_nodes, k);
> -	else {
> +	} else {
>  		child = xbc_node_get_child(last_parent);
>  		/* Since the value node is the first child, skip it. */
>  		if (child && xbc_node_is_value(child))
> @@ -667,9 +667,9 @@ static int __init __xbc_add_key(char *k)
>  		node = find_match_node(child, k);
>  	}
>  
> -	if (node)
> +	if (node) {
>  		last_parent = node;
> -	else {
> +	} else {
>  add_node:
>  		node = xbc_add_child(k, XBC_KEY);
>  		if (!node)
> -- 
> 2.34.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

[Masami Hiramatsu]

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>

Re: [PATCH v4 00/17] bootconfig: fixes, cleanups, and modernization

[Masami Hiramatsu]

Hi Josh,

Thanks for cleaning up. I had some comments. Please check my reply.
Basically, I don't see any urgent bugfixes in this series. In summary;

OK for-next: [01][06][08][09][10][12][13][15]
Need Fixed taa: [16][17]
Request to fold:[02][03][04][05][07]
NACK: [11][14]

Thank you,

On Sat, 14 Mar 2026 23:01:38 +0000
Josh Law <objecting@objecting.org> wrote:

> This series addresses a collection of issues found during a review of
> lib/bootconfig.c, include/linux/bootconfig.h, and tools/bootconfig,
> ranging from off-by-one errors and unchecked return values to coding
> style and API modernization.
> 
> Changes since v3:
>   - Added commit descriptions to all patches that were missing them
>     (patches 2, 3, 4, 7).
>   - Added real-world impact statements to all bug-fix patches
>     (patches 8, 9, 15, 16).
> 
> Changes since v2:
>   - Added "validate child node index in xbc_verify_tree()" —
>     xbc_verify_tree() validated next-node indices but not child indices;
>     an out-of-bounds child would cause xbc_node_get_child() to access
>     memory beyond the xbc_nodes array (patch 15).
>   - Added "check xbc_init_node() return in override path" — the ':='
>     override path in xbc_parse_kv() ignored xbc_init_node()'s return
>     value, silently continuing with stale node data on failure
>     (patch 16).
>   - Added "fix fd leak in load_xbc_file() on fstat failure" — if
>     fstat() failed after open() succeeded, the file descriptor was
>     leaked (patch 17).
> 
> Changes since v1:
>   - Dropped "return empty string instead of NULL from
>     xbc_node_get_data()" — returning "" causes false matches in
>     xbc_node_match_prefix() because strncmp(..., "", 0) always
>     returns 0.
> 
> Bug fixes:
>   - Fix off-by-one in xbc_verify_tree() where a next-node index equal
>     to xbc_node_num passes the bounds check despite being out of range;
>     a malformed bootconfig could cause an out-of-bounds read of kernel
>     memory during tree traversal at boot time (patch 8).
>   - Move xbc_node_num increment to after xbc_init_node() validation
>     so a failed init does not leave a partially initialized node
>     counted in the array; on a maximum-size bootconfig, the
>     uninitialized node could be traversed leading to unpredictable
>     boot behavior (patch 9).
>   - Validate child node indices in xbc_verify_tree() alongside the
>     existing next-node check; without this, a corrupt bootconfig could
>     trigger an out-of-bounds memory access via an invalid child index
>     during tree traversal (patch 15).
>   - Check xbc_init_node() return value in the ':=' override path; a
>     bootconfig using ':=' near the 32KB data limit could silently
>     retain the old value, meaning a security-relevant boot parameter
>     override would not take effect (patch 16).
>   - Fix file descriptor leak in tools/bootconfig load_xbc_file()
>     when fstat() fails (patch 17).
> 
> Correctness:
>   - Add missing __init annotations to skip_comment() and
>     skip_spaces_until_newline() so their memory can be reclaimed
>     after init (patch 1).
>   - Narrow the flag parameter in node creation helpers from uint32_t
>     to uint16_t to match the xbc_node.data field width (patch 6).
>   - Constify the xbc_calc_checksum() data parameter since it only
>     reads the buffer (patch 12).
> 
> Cleanups:
>   - Fix comment typos (patches 2-3), missing blank line before
>     kerneldoc (patch 4), inconsistent if/else bracing (patches 5, 7).
>   - Drop redundant memset after memblock_alloc which already returns
>     zeroed memory; switch the userspace path from malloc to calloc
>     to match (patch 10).
> 
> Modernization:
>   - Replace open-coded __attribute__((__packed__)) with the __packed
>     macro, adding the definition to the tools/bootconfig shim header
>     (patches 11, 14).
>   - Replace the catch-all linux/kernel.h include with the specific
>     headers needed: linux/cache.h, linux/compiler.h, and
>     linux/sprintf.h (patch 13).
> 
> Build-tested with both the in-kernel build (lib/bootconfig.o,
> init/main.o) and the userspace tools/bootconfig build. All 70
> tools/bootconfig test cases pass.
> 
> Josh Law (17):
>   lib/bootconfig: add missing __init annotations to static helpers
>   lib/bootconfig: fix typo "initiized" in xbc_root_node() kerneldoc
>   lib/bootconfig: fix typo "uder" in xbc_node_find_next_leaf()
>   lib/bootconfig: add blank line before xbc_get_info() kerneldoc
>   lib/bootconfig: fix inconsistent if/else bracing
>   lib/bootconfig: narrow flag parameter type from uint32_t to uint16_t
>   lib/bootconfig: fix inconsistent if/else bracing in __xbc_add_key()
>   lib/bootconfig: fix off-by-one in xbc_verify_tree() next node check
>   lib/bootconfig: increment xbc_node_num after node init succeeds
>   lib/bootconfig: drop redundant memset of xbc_nodes
>   bootconfig: use __packed macro for struct xbc_node
>   bootconfig: constify xbc_calc_checksum() data parameter
>   lib/bootconfig: replace linux/kernel.h with specific includes
>   bootconfig: add __packed definition to tools/bootconfig shim header
>   lib/bootconfig: validate child node index in xbc_verify_tree()
>   lib/bootconfig: check xbc_init_node() return in override path
>   tools/bootconfig: fix fd leak in load_xbc_file() on fstat failure
> 
>  include/linux/bootconfig.h                  |  6 +--
>  lib/bootconfig.c                            | 54 ++++++++++++---------
>  tools/bootconfig/include/linux/bootconfig.h |  1 +
>  tools/bootconfig/main.c                     |  4 +-
>  4 files changed, 39 insertions(+), 26 deletions(-)
> 
> -- 
> 2.34.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>