[PATCH] bootm: fix boot override inheritance

[PATCH] bootm: fix boot override inheritance

[Ahmad Fatoum]

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>
---
 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,
-- 
2.47.3

Re: [PATCH] bootm: fix boot override inheritance

[Ahmad Fatoum]

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 |