[PATCH v5 00/23] bootconfig: fixes, cleanups, and modernization

[Josh Law <objecting@objecting.org>]

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, signedness/type cleanup, and API modernization.

Changes since v4:
  - Added six follow-up patches found via static analysis with strict
    GCC warnings (patches 18-23).
  - Added "fix signed comparison in xbc_node_get_data()" -- switch the
    masked offset variable to unsigned int and compare against
    XBC_DATA_MAX to avoid a signed comparison and make the mask
    self-documenting (patch 18).
  - Added "use size_t for strlen result in xbc_node_match_prefix()"
    and "use size_t for key length tracking in xbc_verify_tree()" to
    match strlen() return types (patches 19, 21).
  - Added "narrow offset type in xbc_init_node()" -- use a validated
    unsigned int temporary for the stored 15-bit data offset
    (patch 20).
  - Added "fix sign-compare in xbc_node_compose_key_after()" -- cast
    the checked snprintf() return when comparing and subtracting
    against a size_t buffer length (patch 22).
  - Added "change xbc_node_index() return type to uint16_t" -- match
    the 16-bit storage fields and XBC_NODE_MAX bounds (patch 23).

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).
  - Fix strict-GCC signedness and narrowing warnings by aligning local
    types with strlen()/snprintf() APIs and the 16-bit node index/data
    storage in xbc_node_get_data(), xbc_node_match_prefix(),
    xbc_init_node(), xbc_verify_tree(), xbc_node_compose_key_after(),
    and xbc_node_index() (patches 18-23).

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 (23):
  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
  lib/bootconfig: fix signed comparison in xbc_node_get_data()
  lib/bootconfig: use size_t for strlen result in
    xbc_node_match_prefix()
  lib/bootconfig: narrow offset type in xbc_init_node()
  lib/bootconfig: use size_t for key length tracking in
    xbc_verify_tree()
  lib/bootconfig: fix sign-compare in xbc_node_compose_key_after()
  lib/bootconfig: change xbc_node_index() return type to uint16_t

 include/linux/bootconfig.h                  |  8 +--
 lib/bootconfig.c                            | 71 ++++++++++++---------
 tools/bootconfig/include/linux/bootconfig.h |  1 +
 tools/bootconfig/main.c                     |  4 +-
 4 files changed, 49 insertions(+), 35 deletions(-)

-- 
2.34.1