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

Reply via email to