[PATCH 09/11] boot: bootm: release the imagemap on every failing exit

[Simon Glass]

Hi Daniel,

On 2026-09-29T00:04:16, Daniel Golle <daniel at makrotopia.org> wrote:
> boot: bootm: release the imagemap on every failing exit
>
> Three error paths in bootm_run_states() return directly instead of
> reaching the err label, so the imagemap device and the LMB reservations
> its translation table holds survive a failed bootm: the missing OS boot
> function, an unsupported subcommand, and any error carried out of the
> earlier states. The first of those also left interrupts disabled.
>
> Route all three through err, which already releases the imagemap and
> re-enables interrupts.
>
> Fixes: 46d32e38ee4e ("boot: fit: support on-demand loading in fit_image_load()")
> Signed-off-by: Daniel Golle <daniel at makrotopia.org>
>
> boot/bootm.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

> The first of those also left interrupts disabled.

Just to check: iflag is set as soon as BOOTM_STATE_LOADOS runs, so a
later ramdisk/FDT failure or a failed subcommand also returned with
interrupts disabled. All three paths had this problem, not just the
first.

> diff --git a/boot/bootm.c b/boot/bootm.c
> @@ -1265,18 +1265,17 @@ int bootm_run_states(struct bootm_info *bmi, int states)
>       /* From now on, we need the OS boot function */
>       if (ret)
> -             return ret;
> +             goto err;

The err label does more than release resources. It also handles
BOOTM_ERR_RESET / BOOTM_ERR_UNIMPLEMENTED, which was only meant for
errors from bootm_load_os(). BOOTM_ERR_RESET is -1, and
boot_ramdisk_high() returns -1 on any failure, so with this patch a
'ramdisk - allocation error' or 'in-place initrd alloc failed' now
prints 'Resetting the board...' and calls reset_cpu() instead of
returning the error. The same applies to the subcommand path below: on
ARM, do_bootm_linux() returns -1 for BOOTM_STATE_OS_BD_T and
BOOTM_STATE_OS_CMDLINE, so 'bootm bdt' would reset the board. This
happens whether or not an imagemap is in use.

Please can you keep the reset/unimplemented handling for the load_os
path only? One way is to give that path its own label that falls
through to the common cleanup. Another is to move the imagemap release
and enable_interrupts() into a separate label after the BOOTM_ERR_*
checks, and send these three exits there. What do you think?

> diff --git a/boot/bootm.c b/boot/bootm.c
> @@ -1318,7 +1317,7 @@ int bootm_run_states(struct bootm_info *bmi, int states)
>       if (ret) {
>               printf("subcommand failed (err=%d)\n", ret);
> -             return ret;
> +             goto err;
>       }

See above. It would also be worth adding a sandbox test for a failing
bootm subcommand with an imagemap attached, so this path is covered.

Regards,
Simon