Re: [PATCH] bootm: fix boot override inheritance

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 11/10/25 2:33 PM, Ahmad Fatoum wrote:
> Nested boot calls were incorrectly handled by the override mechanism,
> although they are a common occurrence e.g. when using bootchooser:
> 
>  - boot command enumerates entries including bootchooser and boots the
>    "bootchooser" entry
>  - bootchooser boot enumerates entries for its targets and boots
>    one of these entries
> 
> Each boot entry boot calls bootm_set_overrides(), which is broken in the
> nested case for two reasons:
> 
>  - bootm_set_overrides(&be->overrides) reverts the overrides for the
>    first nested boot entry, because it doesn't first check if the
>    individual overrides (as opposed to the pointer to the struct
>    containing them) is NULL
> 
>  - bootm_set_overrides(NULL) additionally reverts the overrides for
>    all later boot entries, because instead of restoring the previous
>    override state, it discards it.
> 
> Fix both of these by having bootm_set_overrides return the old
> overrides, so they can be reinstated.
> 
> Reported-by: Ulrich Ölmann <u.oelmann@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

While this is a fix, I don't think it's critical for this month's
release, so it can go into master after.

Cheers,
Ahmad

> ---
>  common/boot.c             | 5 +++--
>  common/bootm.c            | 7 +++++--
>  include/bootm-overrides.h | 7 +++++--
>  3 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/common/boot.c b/common/boot.c
> index d755805d9314..07bd288df29d 100644
> --- a/common/boot.c
> +++ b/common/boot.c
> @@ -156,6 +156,7 @@ BAREBOX_MAGICVAR(global.boot.watchdog_timeout,
>  
>  int boot_entry(struct bootentry *be, int verbose, int dryrun)
>  {
> +	struct bootm_overrides old;
>  	int ret;
>  
>  	pr_info("Booting entry '%s'\n", be->title);
> @@ -170,13 +171,13 @@ int boot_entry(struct bootentry *be, int verbose, int dryrun)
>  		}
>  	}
>  
> -	bootm_set_overrides(&be->overrides);
> +	old = bootm_set_overrides(be->overrides);
>  
>  	ret = be->boot(be, verbose, dryrun);
>  	if (ret && ret != -ENOMEDIUM)
>  		pr_err("Booting entry '%s' failed: %pe\n", be->title, ERR_PTR(ret));
>  
> -	bootm_set_overrides(NULL);
> +	bootm_set_overrides(old);
>  
>  	globalvar_set_match("linux.bootargs.dyn.", "");
>  
> diff --git a/common/bootm.c b/common/bootm.c
> index 17792b2a1d81..37c09b846f2a 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -1010,9 +1010,12 @@ int bootm_boot(struct bootm_data *bootm_data)
>  }
>  
>  #ifdef CONFIG_BOOT_OVERRIDE
> -void bootm_set_overrides(const struct bootm_overrides *overrides)
> +struct bootm_overrides bootm_set_overrides(const struct bootm_overrides overrides)
>  {
> -	bootm_overrides = overrides ? *overrides : (struct bootm_overrides){};
> +	struct bootm_overrides old = bootm_overrides;
> +	/* bootm_merge_overrides copies only actual (non-NULL) overrides */
> +	bootm_merge_overrides(&bootm_overrides, &overrides);
> +	return old;
>  }
>  #endif
>  
> diff --git a/include/bootm-overrides.h b/include/bootm-overrides.h
> index 19557f63acd9..4a270b95afcc 100644
> --- a/include/bootm-overrides.h
> +++ b/include/bootm-overrides.h
> @@ -8,9 +8,12 @@ struct bootm_overrides {
>  };
>  
>  #ifdef CONFIG_BOOT_OVERRIDE
> -void bootm_set_overrides(const struct bootm_overrides *overrides);
> +struct bootm_overrides bootm_set_overrides(const struct bootm_overrides overrides_new);
>  #else
> -static inline void bootm_set_overrides(const struct bootm_overrides *overrides) {}
> +static inline struct bootm_overrides bootm_set_overrides(const struct bootm_overrides overrides)
> +{
> +	return (struct bootm_overrides) {};
> +}
>  #endif
>  
>  static inline void bootm_merge_overrides(struct bootm_overrides *dst,

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |