mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/3] defaultenv: skip non-existent dirs in defaultenv_append_runtime_directory
@ 2025-11-24 17:59 Ahmad Fatoum
  2025-11-24 17:59 ` [PATCH 2/3] defaultenv: don't abort defaultenv_load silently on error Ahmad Fatoum
  2025-11-24 17:59 ` [PATCH 3/3] defaultenv: add base and external defaultenv in pure_initcall Ahmad Fatoum
  0 siblings, 2 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2025-11-24 17:59 UTC (permalink / raw)
  To: barebox; +Cc: Jan Lübbe, Ahmad Fatoum

QEMU fw_cfg file system driver is the sole user of
defaultenv_append_runtime_directory() and it passes a path that's not
necessarily available.

When that happens, this is not noticed until the loop in defaultenv_load()
aborts early with an -ENOENT preventing load of an external environment.

Fix this as well as further issues of this sort by not adding directories
that don't exist at the time defaultenv_append_runtime_directory() is
called.

Fixes: 31dfb561d1a4 ("fs: qemu_fw_cfg: support populating environment via QEMU fw_cfg")
Reported-by: Jan Lübbe <j.luebbe@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 defaultenv/defaultenv.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/defaultenv/defaultenv.c b/defaultenv/defaultenv.c
index d6cca466a683..652fda05027a 100644
--- a/defaultenv/defaultenv.c
+++ b/defaultenv/defaultenv.c
@@ -20,6 +20,7 @@
 #include <init.h>
 #include <libfile.h>
 #include <asm/unaligned.h>
+#include <fs.h>
 #include "barebox_default_env.h"
 
 static LIST_HEAD(defaultenv_list);
@@ -100,6 +101,10 @@ void defaultenv_append(void *buf, unsigned int size, const char *name)
 void defaultenv_append_runtime_directory(const char *srcdir)
 {
 	struct defaultenv *df;
+	struct stat s;
+
+	if (stat(srcdir, &s) || !S_ISDIR(s.st_mode))
+		return;
 
 	defaultenv_add_base();
 
-- 
2.47.3




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/3] defaultenv: don't abort defaultenv_load silently on error
  2025-11-24 17:59 [PATCH 1/3] defaultenv: skip non-existent dirs in defaultenv_append_runtime_directory Ahmad Fatoum
@ 2025-11-24 17:59 ` Ahmad Fatoum
  2025-11-24 17:59 ` [PATCH 3/3] defaultenv: add base and external defaultenv in pure_initcall Ahmad Fatoum
  1 sibling, 0 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2025-11-24 17:59 UTC (permalink / raw)
  To: barebox; +Cc: Jan Lübbe, Ahmad Fatoum

Failure to load one defaultenv has no bearing on whether subsequent
defaultenv loads would succeed, so do not abort the loop early in that
case.

Reported-by: Jan Lübbe <j.luebbe@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/startup.c        | 8 ++++++--
 defaultenv/defaultenv.c | 8 ++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/common/startup.c b/common/startup.c
index dfea8394fdee..8f5e77d4619c 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -92,8 +92,12 @@ static int load_environment(void)
 
 	default_environment_path = default_environment_path_get();
 
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT))
-		defaultenv_load("/env", 0);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT)) {
+		ret = defaultenv_load("/env", 0);
+		if (ret)
+			pr_warn("Failed loading (some) defaultenv overlays: %pe\n",
+				ERR_PTR(ret));
+	}
 
 	if (IS_ENABLED(CONFIG_ENV_HANDLING)) {
 		envfs_load(default_environment_path, "/env", 0);
diff --git a/defaultenv/defaultenv.c b/defaultenv/defaultenv.c
index 652fda05027a..8407a2457630 100644
--- a/defaultenv/defaultenv.c
+++ b/defaultenv/defaultenv.c
@@ -177,17 +177,17 @@ static int defaultenv_load_one(struct defaultenv *df, const char *dir,
 int defaultenv_load(const char *dir, unsigned flags)
 {
 	struct defaultenv *df;
-	int ret;
+	int err = 0;
 
 	defaultenv_add_base();
 
 	defaultenv_add_external();
 
 	list_for_each_entry(df, &defaultenv_list, list) {
-		ret = defaultenv_load_one(df, dir, flags);
+		int ret = defaultenv_load_one(df, dir, flags);
 		if (ret)
-			return ret;
+			err = ret;
 	}
 
-	return 0;
+	return err;
 }
-- 
2.47.3




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 3/3] defaultenv: add base and external defaultenv in pure_initcall
  2025-11-24 17:59 [PATCH 1/3] defaultenv: skip non-existent dirs in defaultenv_append_runtime_directory Ahmad Fatoum
  2025-11-24 17:59 ` [PATCH 2/3] defaultenv: don't abort defaultenv_load silently on error Ahmad Fatoum
@ 2025-11-24 17:59 ` Ahmad Fatoum
  1 sibling, 0 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2025-11-24 17:59 UTC (permalink / raw)
  To: barebox; +Cc: Jan Lübbe, Ahmad Fatoum

We currently depend on the order of the calls to defaultenv_append() to
decide which overlays to apply first.

To ensure the base environment is always loaded first, all of
defaultenv_append(), defaultenv_append_runtime_directory() and
defaultenv_load() call defaultenv_add_base() first.

Simplify this by adding base and external (i.e. from the BSP) defaultenv
in the earliest initcall.

This has a welcome side effect: Previously, fw_cfg env was applied
before an external defaultenv, but now it's applied afterwards and thus
can override it.

Reported-by: Jan Lübbe <j.luebbe@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 defaultenv/defaultenv.c | 78 +++++++++++++++--------------------------
 1 file changed, 28 insertions(+), 50 deletions(-)

diff --git a/defaultenv/defaultenv.c b/defaultenv/defaultenv.c
index 8407a2457630..6891d95e301e 100644
--- a/defaultenv/defaultenv.c
+++ b/defaultenv/defaultenv.c
@@ -33,48 +33,6 @@ struct defaultenv {
 	size_t size;
 };
 
-static void defaultenv_add_base(void)
-{
-	static int base_added;
-
-	if (base_added)
-		return;
-
-	base_added = 1;
-
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW))
-		defaultenv_append_directory(defaultenv_2_base);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_MENU))
-		defaultenv_append_directory(defaultenv_2_menu);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_DFU))
-		defaultenv_append_directory(defaultenv_2_dfu);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_REBOOT_MODE))
-		defaultenv_append_directory(defaultenv_2_reboot_mode);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_SECURITY_POLICY))
-		defaultenv_append_directory(defaultenv_2_security_policy);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_IKCONFIG))
-		defaultenv_append_directory(defaultenv_2_ikconfig);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC))
-		defaultenv_append_directory(defaultenv_1);
-}
-
-static void defaultenv_add_external(void)
-{
-	static int external_added;
-
-	if (external_added)
-		return;
-
-	external_added = 1;
-
-	/*
-	 * The traditional or external environment given with
-	 * CONFIG_DEFAULT_ENVIRONMENT_PATH.
-	 */
-	defaultenv_append((void *)default_environment,
-			default_environment_size, "defaultenv");
-}
-
 /*
  * defaultenv_append - append a envfs buffer to the default environment
  * @buf:	The buffer containing the binary environment. If it is
@@ -88,8 +46,6 @@ void defaultenv_append(void *buf, unsigned int size, const char *name)
 {
 	struct defaultenv *df;
 
-	defaultenv_add_base();
-
 	df = xzalloc(sizeof(*df));
 	df->buf = buf;
 	df->size = size;
@@ -106,8 +62,6 @@ void defaultenv_append_runtime_directory(const char *srcdir)
 	if (stat(srcdir, &s) || !S_ISDIR(s.st_mode))
 		return;
 
-	defaultenv_add_base();
-
 	df = xzalloc(sizeof(*df));
 	df->srcdir = srcdir;
 	df->name = srcdir;
@@ -179,10 +133,6 @@ int defaultenv_load(const char *dir, unsigned flags)
 	struct defaultenv *df;
 	int err = 0;
 
-	defaultenv_add_base();
-
-	defaultenv_add_external();
-
 	list_for_each_entry(df, &defaultenv_list, list) {
 		int ret = defaultenv_load_one(df, dir, flags);
 		if (ret)
@@ -191,3 +141,31 @@ int defaultenv_load(const char *dir, unsigned flags)
 
 	return err;
 }
+
+static int defaultenv_init(void)
+{
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW))
+		defaultenv_append_directory(defaultenv_2_base);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_MENU))
+		defaultenv_append_directory(defaultenv_2_menu);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_DFU))
+		defaultenv_append_directory(defaultenv_2_dfu);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_REBOOT_MODE))
+		defaultenv_append_directory(defaultenv_2_reboot_mode);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_SECURITY_POLICY))
+		defaultenv_append_directory(defaultenv_2_security_policy);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_IKCONFIG))
+		defaultenv_append_directory(defaultenv_2_ikconfig);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC))
+		defaultenv_append_directory(defaultenv_1);
+
+	/*
+	 * The traditional or external environment given with
+	 * CONFIG_DEFAULT_ENVIRONMENT_PATH.
+	 */
+	defaultenv_append((void *)default_environment,
+			default_environment_size, "defaultenv");
+
+	return 0;
+}
+pure_initcall(defaultenv_init);
-- 
2.47.3




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-11-24 18:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-24 17:59 [PATCH 1/3] defaultenv: skip non-existent dirs in defaultenv_append_runtime_directory Ahmad Fatoum
2025-11-24 17:59 ` [PATCH 2/3] defaultenv: don't abort defaultenv_load silently on error Ahmad Fatoum
2025-11-24 17:59 ` [PATCH 3/3] defaultenv: add base and external defaultenv in pure_initcall Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox