[PATCH v1 0/7] bootstd: Fix crashes and memory leaks on scan/boot error paths

Aristo Chen via U-Boot <[email protected]>
Newsgroups gmane.comp.boot-loaders.u-boot
Message-ID <20260813155617.184928-1-aristo.chen__13023.0159776815$1786643976$gmane$org@canonical.com>
This series fixes a crash and several memory-safety problems in the
bootstd scan and boot error paths, most of them in the RAUC bootmeth.

The starting point is a NULL pointer dereference in the RAUC bootmeth's
get_slot(): its loop condition dereferences each slots[] entry before
checking it against the NULL terminator, so a BOOT_ORDER environment
variable naming a slot that is not configured in
CONFIG_BOOTMETH_RAUC_PARTITIONS crashes U-Boot. BOOT_ORDER is written by
the OS (RAUC) into an often disk-resident environment, so a stray or
corrupted value must not crash the bootloader (patch 1).

The regression test added for that fix then exposed a chain of further
problems on the same paths:

- The RAUC scan error path frees its private data but leaves
  bflow->bootmeth_priv pointing at the freed memory. With
  'bootflow scan -a' the failed bootflow is stored, and removing it
  later frees the pointer a second time (patch 2). The ChromiumOS
  bootmeth has the same problem on its cros_read_info() error path
  (patch 4), and the RAUC boot path has it as well once the boot script
  returns (patch 7). bootmeth_android already clears the pointer; these
  three patches follow that pattern.

- Several RAUC error paths return early without freeing their
  str_to_list() string lists (patch 3), and the strsep() parsing in
  distro_rauc_read_bootflow() leaks its two strdup'd source buffers on
  every call: strsep() advances the pointers to NULL, so the error-path
  free() calls are no-ops and the success path does not free at all.
  This also removes an interior-pointer free() that would corrupt the
  heap if the two configured lists ever had different lengths
  (patch 5).

- The bootstd scan loop abandons every failed bootflow candidate
  without freeing it: the next candidate's bootflow_init() memsets the
  struct and orphans the previous attempt's allocations, starting with
  the name allocated in bootdev_find_in_blk(). A single failing scan of
  a RAUC-style MMC leaks about 1 KB across ~30 abandoned candidates,
  and scans can be retried indefinitely from the U-Boot prompt
  (patch 6). This is the only change outside the bootmeths; bootflows
  returned to the caller (BOOTFLOWIF_ALL) or stored via
  bootstd_add_bootflow() are not affected.

Patches 1, 2 and 6 extend the bootflow_rauc test alongside their
fixes: scanning with an unknown slot in BOOT_ORDER (SIGSEGV without
patch 1), rescanning after a failing 'bootflow scan -a' (double free
without patch 2), and a ut_check_delta() check that repeating a
failing scan does not change the number of allocated bytes. That last
check also covers the leaks fixed in patches 3 and 5. The cros fix
(patch 4) and the RAUC boot-path fix (patch 7) come without tests
since sandbox cannot reach those error paths: there is no
crafted-image fixture for the cros case, and env_save() fails on the
sandbox 'nowhere' environment before the RAUC boot path reaches the
free.

Tested with the full sandbox unit-test suite (test/py -k test_ut, 951
tests) and the bootstd, bootflow, bootdev, bootmeth, vbe, cros, android
and efi_bootmgr pytest suites, all passing with the series applied.
All patches are checkpatch-clean.

Aristo Chen (7):
  bootstd: rauc: Fix NULL dereference in get_slot()
  bootstd: rauc: Clear bootmeth_priv after freeing it on error
  bootstd: rauc: Free string lists on error paths
  bootstd: cros: Clear bootmeth_priv after freeing it on error
  bootstd: rauc: Fix leak of the strsep() source buffers
  bootstd: Free abandoned bootflows while scanning
  bootstd: rauc: Clear bootmeth_priv after freeing it when booting

 boot/bootflow.c      |  2 ++
 boot/bootmeth_cros.c |  1 +
 boot/bootmeth_rauc.c | 37 +++++++++++++++++++++++++++++--------
 test/boot/bootflow.c | 37 +++++++++++++++++++++++++++++++++++++
 4 files changed, 69 insertions(+), 8 deletions(-)

-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.