[PATCH v5 00/11] Add FIT image overlay support

[PATCH v5 00/11] Add FIT image overlay support

[Marco Felsch]

Hi,

this series add the support to load FIT image supplied devicetree overlays.

The overlay loading wasn't coupled to bootm due to the following
reasons:
 - Users can add their own overlay filter
 - By making use of the common overlay handling we can specifiy a
   different/separate FIT image which provides only overlays.
 - It should be possible to apply FIT image overlay to the barebox
   live-tree (not implemented yet).

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
Changes in v5:
- Link to v4: https://lore.barebox.org/20250729-v2024-05-0-topic-fit-overlay-v4-0-af3ad99acde2@pengutronix.de
- fail if CONFIG_FITIMAGE=n and fit overlay was requested
- of_overlay_global_fixup(): adapt pr_err() if stat() fails
- of_overlay_global_fixup(): Drop duplicated pr_err() if FIT overlay load fails
- of_overlay_global_fixup(): Drop filetype detection
- devicetree.rst: Fix documentation

Changes in v4:
- Link to v3: https://lore.barebox.org/20240703-v2024-05-0-topic-fit-overlay-v3-0-c1fd766fd31d@pengutronix.de
- resolve possible filename links to make caching more robust

Changes in v3:
- Link to v2: https://lore.barebox.org/barebox/20240613125818.30499-1-m.felsch@pengutronix.de/
- drop of.overlay.fitconfigpattern usage and make use of an match-fn
  instead
- fit_get_handle(): make use of strcmp
- open_fits: make it static
- fit_close(): use free unconditional

Changes in v2:
- Link to v1: https://lore.pengutronix.de/barebox/20240322164953.1772129-1-m.felsch@pengutronix.de/
- add caching to improve the overlay load time if the same FIT, which
  provides the overlays, provides the kernel, of, initrd.

---
Marco Felsch (11):
      FIT: fix missing free in fit_open error path
      FIT: fit_open_configuration: add match function support
      of: overlay: make the pattern match function more generic
      of: overlay: make search dir more generic
      of: overlay: refactor of_overlay_global_fixup
      FIT: make fit_config_verify_signature public
      of: overlay: add FIT image overlay support
      of: overlay: replace filename with an more unique name
      FIT: fit_open: make filename handling more robust
      FIT: fit_open: save the filename
      FIT: add support to cache opened fit images

 Documentation/user/devicetree.rst |  27 ++---
 common/bootm.c                    |  13 ++-
 common/image-fit.c                |  76 ++++++++++++--
 drivers/of/overlay.c              | 211 ++++++++++++++++++++++++++++++--------
 include/image-fit.h               |  10 +-
 include/of.h                      |   3 +-
 6 files changed, 275 insertions(+), 65 deletions(-)
---
base-commit: 89bf1fcc998fc5fea0ce613d9930dd9ee39c0fb2
change-id: 20240703-v2024-05-0-topic-fit-overlay-76d5176c1d4e

Best regards,
-- 
Marco Felsch <m.felsch@pengutronix.de>

[PATCH v5 01/11] FIT: fix missing free in fit_open error path

[Marco Felsch]

Free the handle if read_file_2() fails.

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/image-fit.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/image-fit.c b/common/image-fit.c
index 5006394eb7bbd0873a37a0102d5a0d89ea7c6b9f..4d5c5bfe12472af85b26cd1ee93e7c99316b283c 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -977,6 +977,7 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
 			  max_size);
 	if (ret && ret != -EFBIG) {
 		pr_err("unable to read %s: %pe\n", filename, ERR_PTR(ret));
+		free(handle);
 		return ERR_PTR(ret);
 	}
 

-- 
2.39.5

[PATCH v5 02/11] FIT: fit_open_configuration: add match function support

[Marco Felsch]

The FIT spec is not very specific and was growing over the past years,
e.g. according U-Boot (doc/usage/fit/source_file_format.rst) either a
"kernel" or "firmware" property is mandatory whereas the fit-overlay
example (doc/usage/fit/overlay-fdt-boot.rst) doesn't fulfil this
requirement. Instead it allows nodes which contain only a "fdt"
property.

This inconsistency makes it hard to detect the purpose of a
configuration node. So far we have three different configuration nodes

- bootable nodes (the usual use-case):

|	config-0 {
|		compatible = "machine-compatible";
|		kernel = "kernel-img-name";
|		fdt = "fdt-img-name";
|	}

- firmware only nodes like (doc/usage/fit/sec_firmware_ppa.rst):

|	config-1 {
|		description = "PPA Secure firmware";
|		firmware = "firmware@1";
|		loadables = "trustedOS@1", "fuse_scr";
|	};

- overlay only nodes:

|	config-2 {
|		fdt = "fdt-overlay-img-name";
|	}

This commit adds an optional match function which can be passed to the
fit_open_configuration() to sort out config nodes which are not
interessting, e.g. the bootm code is only interested in config nodes
which do provide a "kernel" image.

This new match function gets called if no explicit configuration node
name was specified.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/bootm.c      | 13 ++++++++++++-
 common/image-fit.c  | 22 ++++++++++++++++------
 include/image-fit.h |  4 +++-
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/common/bootm.c b/common/bootm.c
index 95211011fe8bf5f52ce3155de419bc9b09ff2228..7f37c74eb41e5c7a89aa4038bae46cee5f056f5f 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -609,6 +609,16 @@ static int bootm_open_os_uimage(struct image_data *data)
 	return 0;
 }
 
+static bool bootm_fit_config_valid(struct fit_handle *fit,
+				   struct device_node *config)
+{
+	/*
+	 * Consider only FIT configurations which do provide a loadable kernel
+	 * image.
+	 */
+	return !!fit_has_image(fit, config, "kernel");
+}
+
 static int bootm_open_fit(struct image_data *data)
 {
 	struct fit_handle *fit;
@@ -634,7 +644,8 @@ static int bootm_open_fit(struct image_data *data)
 	data->os_fit = fit;
 
 	data->fit_config = fit_open_configuration(data->os_fit,
-						  data->os_part);
+						  data->os_part,
+						  bootm_fit_config_valid);
 	if (IS_ERR(data->fit_config)) {
 		pr_err("Cannot open FIT image configuration '%s'\n",
 		       data->os_part ? data->os_part : "default");
diff --git a/common/image-fit.c b/common/image-fit.c
index 4d5c5bfe12472af85b26cd1ee93e7c99316b283c..0fc3b2e4aa7217a47710ef613426456420e81324 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -782,7 +782,9 @@ static int fit_fdt_is_compatible(struct fit_handle *handle,
 
 static int fit_find_compatible_unit(struct fit_handle *handle,
 				    struct device_node *conf_node,
-				    const char **unit)
+				    const char **unit,
+				    bool (*config_node_valid)(struct fit_handle *handle,
+							      struct device_node *config))
 {
 	struct device_node *child = NULL;
 	struct device_node *barebox_root;
@@ -799,7 +801,12 @@ static int fit_find_compatible_unit(struct fit_handle *handle,
 		return -ENOENT;
 
 	for_each_child_of_node(conf_node, child) {
-		int score = of_device_is_compatible(child, machine);
+		int score;
+
+		if (config_node_valid && !config_node_valid(handle, child))
+			continue;
+
+		score = of_device_is_compatible(child, machine);
 
 		if (!score)
 			score = fit_fdt_is_compatible(handle, child, machine);
@@ -857,7 +864,9 @@ static int fit_find_last_unit(struct fit_handle *handle,
  * Return: If successful a pointer to a valid configuration node,
  *         otherwise a ERR_PTR()
  */
-void *fit_open_configuration(struct fit_handle *handle, const char *name)
+void *fit_open_configuration(struct fit_handle *handle, const char *name,
+			     bool (*match_valid)(struct fit_handle *handle,
+						 struct device_node *config))
 {
 	struct device_node *conf_node = handle->configurations;
 	const char *unit, *desc = "(no description)";
@@ -869,7 +878,8 @@ void *fit_open_configuration(struct fit_handle *handle, const char *name)
 	if (name) {
 		unit = name;
 	} else {
-		ret = fit_find_compatible_unit(handle, conf_node, &unit);
+		ret = fit_find_compatible_unit(handle, conf_node, &unit,
+					       match_valid);
 		if (ret) {
 			pr_info("Couldn't get a valid configuration. Aborting.\n");
 			return ERR_PTR(ret);
@@ -1044,12 +1054,12 @@ static int fuzz_fit(const u8 *data, size_t size)
 	if (ret)
 		goto out;
 
-	config = fit_open_configuration(&handle, NULL);
+	config = fit_open_configuration(&handle, NULL, NULL);
 	if (IS_ERR(config)) {
 		ret = fit_find_last_unit(&handle, &unit);
 		if (ret)
 			goto out;
-		config = fit_open_configuration(&handle, unit);
+		config = fit_open_configuration(&handle, unit, NULL);
 	}
 	if (IS_ERR(config)) {
 		ret = PTR_ERR(config);
diff --git a/include/image-fit.h b/include/image-fit.h
index 0b8e94bf46357a1fe01ef90cd8b0fca0c41641c3..416f1f2c1896949d2bcb7ab1a31f1aa6b5682edf 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -26,7 +26,9 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
 			    enum bootm_verify verify, loff_t max_size);
 struct fit_handle *fit_open_buf(const void *buf, size_t len, bool verbose,
 				enum bootm_verify verify);
-void *fit_open_configuration(struct fit_handle *handle, const char *name);
+void *fit_open_configuration(struct fit_handle *handle, const char *name,
+			     bool (*match_valid)(struct fit_handle *handle,
+						 struct device_node *config));
 int fit_has_image(struct fit_handle *handle, void *configuration,
 		  const char *name);
 int fit_open_image(struct fit_handle *handle, void *configuration,

-- 
2.39.5

[PATCH v5 03/11] of: overlay: make the pattern match function more generic

[Marco Felsch]

The current overlay mechanism can handle files only, so filepattern was
an obvious name.

Drop the "file" prefix to prepare the codebase to handle overlays
supplied via a FIT images as well.

Keep the backward compatibility by still providing the filepattern
filter and the global of.overlay.filepattern variable, but mark them as
deprecated.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 Documentation/user/devicetree.rst |  6 +++---
 drivers/of/overlay.c              | 37 ++++++++++++++++++++++++++-----------
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/Documentation/user/devicetree.rst b/Documentation/user/devicetree.rst
index ea2945ab2b564a10a780ba0055daa37ed0c85581..ef04e14f7c0dde579493425073f1ee01395f0d8c 100644
--- a/Documentation/user/devicetree.rst
+++ b/Documentation/user/devicetree.rst
@@ -118,15 +118,15 @@ the kernel. The behaviour is controlled by different variables:
   these compatibles will be applied. When this list is empty then all overlays
   will be applied. Overlays that don't have a compatible are considered being
   always compatible.
-``global.of.overlay.filepattern``
+``global.of.overlay.pattern``
   This is a space separated list of file patterns. An overlay is only applied
   when its filename matches one of the patterns. The patterns can contain
   ``*`` and ``?`` as wildcards. The default is ``*`` which means all files are
   applied.
 ``global.of.overlay.filter``
   This is a space separated list of filters to apply. There are two generic filters:
-  ``filepattern`` matches ``global.of.overlay.filepattern`` above, ``compatible`` matches
-  ``global.of.overlay.compatible`` above. The default is ``filepattern compatible``
+  ``pattern`` matches ``global.of.overlay.pattern`` above, ``compatible`` matches
+  ``global.of.overlay.compatible`` above. The default is ``pattern compatible``
   which means the two generic filters are active. This list may be replaced or
   supplemented by board specific filters.
 
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 6944dd4a744de909c89622322c99b81fc046e43b..a63cca1b47e74615d4453a27a55fb15d1f706fae 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -407,7 +407,7 @@ int of_register_overlay(struct device_node *overlay)
 	return of_register_fixup(of_overlay_fixup, overlay);
 }
 
-static char *of_overlay_filepattern;
+static char *of_overlay_pattern;
 static char *of_overlay_dir;
 static char *of_overlay_basedir;
 
@@ -504,10 +504,10 @@ int of_overlay_register_filter(struct of_overlay_filter *filter)
 }
 
 /**
- * of_overlay_filter_filename - A filter that matches on the filename of
+ * of_overlay_filter_pattern - A filter that matches on the filename or
  *                                an overlay
  * @f: The filter
- * @filename: The filename of the overlay
+ * @pattern: The filename of the overlay
  *
  * This filter matches when the filename matches one of the patterns given
  * in global.of.overlay.filepattern. global.of.overlay.filepattern shall
@@ -515,20 +515,20 @@ int of_overlay_register_filter(struct of_overlay_filter *filter)
  *
  * @return: True when the overlay shall be applied, false otherwise.
  */
-static bool of_overlay_filter_filename(struct of_overlay_filter *f,
-				       const char *filename)
+static bool of_overlay_filter_pattern(struct of_overlay_filter *f,
+				      const char *pattern)
 {
 	char *p, *path, *n;
 	int ret;
 	bool apply;
 
-	p = path = strdup(of_overlay_filepattern);
+	p = path = strdup(of_overlay_pattern);
 
 	while ((n = strsep_unescaped(&p, " ", NULL))) {
 		if (!*n)
 			continue;
 
-		ret = fnmatch(n, filename, 0);
+		ret = fnmatch(n, pattern, 0);
 
 		if (!ret) {
 			apply = true;
@@ -543,6 +543,18 @@ static bool of_overlay_filter_filename(struct of_overlay_filter *f,
 	return apply;
 }
 
+static struct of_overlay_filter of_overlay_pattern_filter = {
+	.name = "pattern",
+	.filter_filename = of_overlay_filter_pattern,
+};
+
+static bool of_overlay_filter_filename(struct of_overlay_filter *f,
+				       const char *filename)
+{
+	pr_warn("'filepattern' filter is marked as deprecated, convert to 'pattern' filter\n");
+	return of_overlay_filter_pattern(f, filename);
+}
+
 static struct of_overlay_filter of_overlay_filepattern_filter = {
 	.name = "filepattern",
 	.filter_filename = of_overlay_filter_filename,
@@ -597,15 +609,18 @@ static struct of_overlay_filter of_overlay_compatible_filter = {
 
 static int of_overlay_init(void)
 {
-	of_overlay_filepattern = strdup("*");
-	of_overlay_filter = strdup("filepattern compatible");
+	of_overlay_pattern = strdup("*");
+	of_overlay_filter = strdup("pattern compatible");
 	of_overlay_set_basedir("/");
 
 	globalvar_add_simple_string("of.overlay.compatible", &of_overlay_compatible);
-	globalvar_add_simple_string("of.overlay.filepattern", &of_overlay_filepattern);
+	globalvar_add_simple_string("of.overlay.pattern", &of_overlay_pattern);
 	globalvar_add_simple_string("of.overlay.filter", &of_overlay_filter);
 	globalvar_add_simple_string("of.overlay.dir", &of_overlay_dir);
 
+	globalvar_alias_deprecated("of.overlay.filepattern", "of.overlay.pattern");
+
+	of_overlay_register_filter(&of_overlay_pattern_filter);
 	of_overlay_register_filter(&of_overlay_filepattern_filter);
 	of_overlay_register_filter(&of_overlay_compatible_filter);
 
@@ -616,6 +631,6 @@ static int of_overlay_init(void)
 device_initcall(of_overlay_init);
 
 BAREBOX_MAGICVAR(global.of.overlay.compatible, "space separated list of compatibles an overlay must match");
-BAREBOX_MAGICVAR(global.of.overlay.filepattern, "space separated list of filepatterns an overlay must match");
+BAREBOX_MAGICVAR(global.of.overlay.pattern, "space separated list of filepatterns an overlay must match");
 BAREBOX_MAGICVAR(global.of.overlay.dir, "Directory to look for dt overlays");
 BAREBOX_MAGICVAR(global.of.overlay.filter, "space separated list of filters");

-- 
2.39.5

[PATCH v5 04/11] of: overlay: make search dir more generic

[Marco Felsch]

Rename global.of.overlay.dir to global.of.overlay.path to prepare the
codebase to handle FIT image overlays as well because *.path doesn't
imply that it have to be a directory.

Keep the backward compatibility by providing the of.overlay.dir variable
but mark it as deprecated.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 Documentation/user/devicetree.rst |  2 +-
 drivers/of/overlay.c              | 15 ++++++++-------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/Documentation/user/devicetree.rst b/Documentation/user/devicetree.rst
index ef04e14f7c0dde579493425073f1ee01395f0d8c..30a15a8865c154f8afe23d641da10d0afa84423b 100644
--- a/Documentation/user/devicetree.rst
+++ b/Documentation/user/devicetree.rst
@@ -108,7 +108,7 @@ Device tree overlays on the kernel device tree
 Overlays can be applied to the kernel device tree before it is handed over to
 the kernel. The behaviour is controlled by different variables:
 
-``global.of.overlay.dir``
+``global.of.overlay.path``
   Overlays are read from this directory. barebox will try to apply all overlays
   found here if not limited by one of the other variables below. When the path
   given here is an absolute path it is used as is. A relative path is relative
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index a63cca1b47e74615d4453a27a55fb15d1f706fae..bc3fdff0cd277b3460981769c2b220adfced6e80 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -408,7 +408,7 @@ int of_register_overlay(struct device_node *overlay)
 }
 
 static char *of_overlay_pattern;
-static char *of_overlay_dir;
+static char *of_overlay_path;
 static char *of_overlay_basedir;
 
 /**
@@ -468,13 +468,13 @@ static int of_overlay_global_fixup(struct device_node *root, void *data)
 	char *dir;
 	int ret;
 
-	if (*of_overlay_dir == '/')
-		return of_overlay_apply_dir(root, of_overlay_dir, true);
+	if (*of_overlay_path == '/')
+		return of_overlay_apply_dir(root, of_overlay_path, true);
 
-	if (*of_overlay_dir == '\0')
+	if (*of_overlay_path == '\0')
 		return 0;
 
-	dir = concat_path_file(of_overlay_basedir, of_overlay_dir);
+	dir = concat_path_file(of_overlay_basedir, of_overlay_path);
 
 	ret = of_overlay_apply_dir(root, dir, true);
 
@@ -616,9 +616,10 @@ static int of_overlay_init(void)
 	globalvar_add_simple_string("of.overlay.compatible", &of_overlay_compatible);
 	globalvar_add_simple_string("of.overlay.pattern", &of_overlay_pattern);
 	globalvar_add_simple_string("of.overlay.filter", &of_overlay_filter);
-	globalvar_add_simple_string("of.overlay.dir", &of_overlay_dir);
+	globalvar_add_simple_string("of.overlay.path", &of_overlay_path);
 
 	globalvar_alias_deprecated("of.overlay.filepattern", "of.overlay.pattern");
+	globalvar_alias_deprecated("of.overlay.dir", "of.overlay.path");
 
 	of_overlay_register_filter(&of_overlay_pattern_filter);
 	of_overlay_register_filter(&of_overlay_filepattern_filter);
@@ -632,5 +633,5 @@ device_initcall(of_overlay_init);
 
 BAREBOX_MAGICVAR(global.of.overlay.compatible, "space separated list of compatibles an overlay must match");
 BAREBOX_MAGICVAR(global.of.overlay.pattern, "space separated list of filepatterns an overlay must match");
-BAREBOX_MAGICVAR(global.of.overlay.dir, "Directory to look for dt overlays");
+BAREBOX_MAGICVAR(global.of.overlay.path, "Path to look for dt overlays");
 BAREBOX_MAGICVAR(global.of.overlay.filter, "space separated list of filters");

-- 
2.39.5

[PATCH v5 05/11] of: overlay: refactor of_overlay_global_fixup

[Marco Felsch]

Prepare of_overlay_global_fixup() for upcoming FIT image based overlays
support.

Check if the of_overlay_path is empty and return early. Make use of
isempty() helper while on it.

Simplify the code flow by setting up the dir variable accordingly before
calling of_overlay_apply_dir() to drop special '/' path
of_overlay_apply_dir() call.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/of/overlay.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index bc3fdff0cd277b3460981769c2b220adfced6e80..f02a12d44f0d53db3fb7bb065461c0ef193d0ab3 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -468,13 +468,13 @@ static int of_overlay_global_fixup(struct device_node *root, void *data)
 	char *dir;
 	int ret;
 
-	if (*of_overlay_path == '/')
-		return of_overlay_apply_dir(root, of_overlay_path, true);
-
-	if (*of_overlay_path == '\0')
+	if (isempty(of_overlay_path))
 		return 0;
 
-	dir = concat_path_file(of_overlay_basedir, of_overlay_path);
+	if (*of_overlay_path == '/')
+		dir = xstrdup(of_overlay_path);
+	else
+		dir = concat_path_file(of_overlay_basedir, of_overlay_path);
 
 	ret = of_overlay_apply_dir(root, dir, true);
 

-- 
2.39.5

[PATCH v5 06/11] FIT: make fit_config_verify_signature public

[Marco Felsch]

The upcoming FIT overlay support requires this helper, so make it public
available.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/image-fit.c  | 2 +-
 include/image-fit.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index 0fc3b2e4aa7217a47710ef613426456420e81324..0067f46e60bc954b418aef3398e2c10856b41c02 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -694,7 +694,7 @@ int fit_open_image(struct fit_handle *handle, void *configuration,
 	return 0;
 }
 
-static int fit_config_verify_signature(struct fit_handle *handle, struct device_node *conf_node)
+int fit_config_verify_signature(struct fit_handle *handle, struct device_node *conf_node)
 {
 	struct device_node *sig_node;
 	int ret = -EINVAL;
diff --git a/include/image-fit.h b/include/image-fit.h
index 416f1f2c1896949d2bcb7ab1a31f1aa6b5682edf..f8321aa87e11fcea642593e790a37df72a2e93b1 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -37,6 +37,7 @@ int fit_open_image(struct fit_handle *handle, void *configuration,
 int fit_get_image_address(struct fit_handle *handle, void *configuration,
 			  const char *name, const char *property,
 			  unsigned long *address);
+int fit_config_verify_signature(struct fit_handle *handle, struct device_node *conf_node);
 
 void fit_close(struct fit_handle *handle);
 

-- 
2.39.5

[PATCH v5 07/11] of: overlay: add FIT image overlay support

[Marco Felsch]

This adds the support to load devicetree overlays from a FIT image.
There are a few options to handle FIT overlays since the FIT overlay
spec is not very strict.

This implements the most configurable case where each overlay does have
its own config node (including the optional signature).

- The "pattern" filter matches the config-node names (the node names
  below the configurations node), not the overlay image names (the node
  names below the images node).

- The "compatible" filter check doesn't differ from the file based overlay
  handling.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 Documentation/user/devicetree.rst |  19 +++---
 drivers/of/overlay.c              | 126 +++++++++++++++++++++++++++++++++++---
 2 files changed, 128 insertions(+), 17 deletions(-)

diff --git a/Documentation/user/devicetree.rst b/Documentation/user/devicetree.rst
index 30a15a8865c154f8afe23d641da10d0afa84423b..644fb551adcf913ca9ed2ab3f1d5fbe7c367bdf6 100644
--- a/Documentation/user/devicetree.rst
+++ b/Documentation/user/devicetree.rst
@@ -109,20 +109,23 @@ Overlays can be applied to the kernel device tree before it is handed over to
 the kernel. The behaviour is controlled by different variables:
 
 ``global.of.overlay.path``
-  Overlays are read from this directory. barebox will try to apply all overlays
-  found here if not limited by one of the other variables below. When the path
-  given here is an absolute path it is used as is. A relative path is relative
-  to ``/`` or relative to the rootfs when using bootloader spec.
+  Overlays are read from this path. The path can either be a directory which
+  contains the overlays or an absolute path to a FIT-image. barebox will try to
+  apply all overlays found if not limited by one of the other variables below.
+  When the path given here is an absolute path it is used as is. A relative
+  path is relative to ``/`` or relative to the rootfs when using bootloader
+  spec.
 ``global.of.overlay.compatible``
   This is a space separated list of compatibles. Only overlays matching one of
   these compatibles will be applied. When this list is empty then all overlays
   will be applied. Overlays that don't have a compatible are considered being
   always compatible.
 ``global.of.overlay.pattern``
-  This is a space separated list of file patterns. An overlay is only applied
-  when its filename matches one of the patterns. The patterns can contain
-  ``*`` and ``?`` as wildcards. The default is ``*`` which means all files are
-  applied.
+  This is a space separated list of file patterns or FIT-image config-node name
+  patterns. An overlay is only applied when its filename or FIT-image
+  config-node name matches one of the patterns. The patterns can contain ``*``
+  and ``?`` as wildcards. The default is ``*`` which means all files or FIT-Image
+  config-nodes are applied.
 ``global.of.overlay.filter``
   This is a space separated list of filters to apply. There are two generic filters:
   ``pattern`` matches ``global.of.overlay.pattern`` above, ``compatible`` matches
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index f02a12d44f0d53db3fb7bb065461c0ef193d0ab3..6defd6fa6908ff2b2fd00d161464af37ac17e040 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -8,10 +8,12 @@
  */
 #define pr_fmt(fmt) "of_overlay: " fmt
 
+#include <bootm.h>
 #include <common.h>
 #include <of.h>
 #include <errno.h>
 #include <globalvar.h>
+#include <image-fit.h>
 #include <magicvar.h>
 #include <string.h>
 #include <libfile.h>
@@ -463,8 +465,100 @@ static int of_overlay_apply_dir(struct device_node *root, const char *dirname,
 	return ret;
 }
 
+static int of_overlay_apply_fit(struct device_node *root, struct fit_handle *fit,
+				struct device_node *config)
+{
+	const char *name = config->name;
+	struct device_node *overlay;
+	unsigned long ovl_sz;
+	const void *ovl;
+	int ret;
+
+	if (!of_overlay_matches_filter(name, NULL))
+		return 0;
+
+	ret = fit_open_image(fit, config, "fdt", &ovl, &ovl_sz);
+	if (ret)
+		return ret;
+
+	overlay = of_unflatten_dtb(ovl, ovl_sz);
+
+	if (!of_overlay_matches_filter(NULL, overlay)) {
+		ret = 0;
+		goto out;
+	}
+
+	ret = of_overlay_apply_tree(root, overlay);
+	if (ret == -ENODEV)
+		pr_debug("Not applied %s (not compatible)\n", name);
+	else if (ret)
+		pr_err("Cannot apply %s: %s\n", name, strerror(-ret));
+	else
+		pr_info("Applied %s\n", name);
+
+out:
+	of_delete_node(overlay);
+
+	return ret;
+}
+
+static bool of_overlay_valid_config(struct fit_handle *fit,
+				    struct device_node *config)
+{
+	/*
+	 * Either kernel or firmware is marked as mandatory by U-Boot
+	 * (doc/usage/fit/source_file_format.rst) except for overlays
+	 * (doc/usage/fit/overlay-fdt-boot.rst). Therefore we need to ensure
+	 * that only "fdt" config nodes are recognized as overlay config node.
+	 */
+	if (!fit_has_image(fit, config, "fdt") ||
+	    fit_has_image(fit, config, "kernel") ||
+	    fit_has_image(fit, config, "firmware"))
+		return false;
+
+	return true;
+}
+
+static int of_overlay_global_fixup_fit(struct device_node *root,
+				       const char *fit_path, loff_t fit_size)
+{
+	enum bootm_verify verify = bootm_get_verify_mode();
+	struct device_node *conf_node;
+	struct fit_handle *fit;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_FITIMAGE)) {
+		pr_err("FIT based overlay handling requested while CONFIG_FITIMAGE=n\n");
+		return -EINVAL;
+	}
+
+	fit = fit_open(fit_path, 0, verify, fit_size);
+	if (IS_ERR(fit)) {
+		pr_err("Loading FIT image %s failed with: %pe\n", fit_path, fit);
+		return PTR_ERR(fit);
+	}
+
+	for_each_child_of_node(fit->configurations, conf_node) {
+		if (!of_overlay_valid_config(fit, conf_node))
+			continue;
+
+		ret = fit_config_verify_signature(fit, conf_node);
+		if (ret)
+			goto out;
+
+		ret = of_overlay_apply_fit(root, fit, conf_node);
+		if (ret)
+			goto out;
+	}
+
+out:
+	fit_close(fit);
+	return ret;
+}
+
 static int of_overlay_global_fixup(struct device_node *root, void *data)
 {
+	struct stat s;
 	char *dir;
 	int ret;
 
@@ -476,10 +570,24 @@ static int of_overlay_global_fixup(struct device_node *root, void *data)
 	else
 		dir = concat_path_file(of_overlay_basedir, of_overlay_path);
 
-	ret = of_overlay_apply_dir(root, dir, true);
 
-	free(dir);
+	if (stat(dir, &s)) {
+		pr_err("Cannot stat global.of.overlay.path (%s): %s\n",
+		       dir, strerror(errno));
+		ret = -errno;
+		goto out;
+	}
 
+	if (S_ISDIR(s.st_mode)) {
+		ret = of_overlay_apply_dir(root, dir, true);
+		goto out;
+	}
+
+	/* Assume a FIT image if of_overlay_path points to a file */
+	ret = of_overlay_global_fixup_fit(root, dir, s.st_size);
+
+out:
+	free(dir);
 	return ret;
 }
 
@@ -505,13 +613,13 @@ int of_overlay_register_filter(struct of_overlay_filter *filter)
 
 /**
  * of_overlay_filter_pattern - A filter that matches on the filename or
- *                                an overlay
+ *			       FIT config-node name of an overlay
  * @f: The filter
- * @pattern: The filename of the overlay
+ * @pattern: The filename or FIT config-node name of the overlay
  *
- * This filter matches when the filename matches one of the patterns given
- * in global.of.overlay.filepattern. global.of.overlay.filepattern shall
- * contain a space separated list of wildcard patterns.
+ * This filter matches when the filename or FIT config-node name matches one of
+ * the patterns given in global.of.overlay.pattern. global.of.overlay.pattern
+ * shall contain a space separated list of wildcard patterns.
  *
  * @return: True when the overlay shall be applied, false otherwise.
  */
@@ -632,6 +740,6 @@ static int of_overlay_init(void)
 device_initcall(of_overlay_init);
 
 BAREBOX_MAGICVAR(global.of.overlay.compatible, "space separated list of compatibles an overlay must match");
-BAREBOX_MAGICVAR(global.of.overlay.pattern, "space separated list of filepatterns an overlay must match");
-BAREBOX_MAGICVAR(global.of.overlay.path, "Path to look for dt overlays");
+BAREBOX_MAGICVAR(global.of.overlay.pattern, "space separated list of filename or fit config-node name patterns an overlay must match");
+BAREBOX_MAGICVAR(global.of.overlay.path, "Path to look for dt overlays or a path to a FIT image");
 BAREBOX_MAGICVAR(global.of.overlay.filter, "space separated list of filters");

-- 
2.39.5

[PATCH v5 08/11] of: overlay: replace filename with an more unique name

[Marco Felsch]

Since FIT image overlays and file/dir based overlays are supported,
'filename' based variables or function hooks are no longer sufficient.

Rename them to 'pattern' based variables or function hooks. The
filter_filename function hook is still supported, to keep the backward
compatibility, but marked as deprecated.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/of/overlay.c | 39 +++++++++++++++++++++------------------
 include/of.h         |  3 ++-
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 6defd6fa6908ff2b2fd00d161464af37ac17e040..8b5d1fc0e77b4db5fb0b234090bae5b45ae6c04a 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -234,12 +234,12 @@ static struct of_overlay_filter *of_overlay_find_filter(const char *name)
 	return NULL;
 }
 
-static bool of_overlay_matches_filter(const char *filename, struct device_node *ovl)
+static bool of_overlay_matches_filter(const char *pattern, struct device_node *ovl)
 {
 	struct of_overlay_filter *filter;
 	char *p, *path, *n;
 	bool apply = false;
-	bool have_filename_filter = false;
+	bool have_pattern_filter = false;
 	bool have_content_filter = false;
 
 	p = path = strdup(of_overlay_filter);
@@ -256,14 +256,16 @@ static bool of_overlay_matches_filter(const char *filename, struct device_node *
 			continue;
 		}
 
-		if (filter->filter_filename)
-			have_filename_filter = true;
+		if (filter->filter_pattern || filter->filter_filename)
+			have_pattern_filter = true;
 		if (filter->filter_content)
 			have_content_filter = true;
 
-		if (filename) {
-			if (filter->filter_filename &&
-			    filter->filter_filename(filter, kbasename(filename)))
+		if (pattern) {
+			if ((filter->filter_pattern &&
+			     filter->filter_pattern(filter, kbasename(pattern))) ||
+			    (filter->filter_filename &&
+			     filter->filter_filename(filter, kbasename(pattern))))
 				score++;
 		} else {
 			score++;
@@ -286,11 +288,11 @@ static bool of_overlay_matches_filter(const char *filename, struct device_node *
 	free(path);
 
 	/* No filter found at all, no match */
-	if (!have_filename_filter && !have_content_filter)
+	if (!have_pattern_filter && !have_content_filter)
 		return false;
 
-	/* Want to match filename, but we do not have a filename_filter */
-	if (filename && !have_filename_filter)
+	/* Want to match pattern, but we do not have a filename_filter */
+	if (pattern && !have_pattern_filter)
 		return true;
 
 	/* Want to match content, but we do not have a content_filter */
@@ -298,12 +300,12 @@ static bool of_overlay_matches_filter(const char *filename, struct device_node *
 		return true;
 
 	if (apply)
-		pr_debug("filename %s, overlay %p: match against filter %s\n",
-			 filename ?: "<NONE>",
+		pr_debug("pattern %s, overlay %p: match against filter %s\n",
+			 pattern ?: "<NONE>",
 			 ovl, filter->name);
 	else
-		pr_debug("filename %s, overlay %p: no match\n",
-			 filename ?: "<NONE>", ovl);
+		pr_debug("pattern %s, overlay %p: no match\n",
+			 pattern ?: "<NONE>", ovl);
 
 	return apply;
 }
@@ -596,14 +598,15 @@ static int of_overlay_global_fixup(struct device_node *root, void *data)
  * @filter: The new filter
  *
  * Register a new overlay filter. A filter can either match on
- * the filename or on the content of an overlay, but not on both.
+ * a pattern or on the content of an overlay, but not on both.
  * If that's desired two filters have to be registered.
  *
  * @return: 0 for success, negative error code otherwise
  */
 int of_overlay_register_filter(struct of_overlay_filter *filter)
 {
-	if (filter->filter_filename && filter->filter_content)
+	if ((filter->filter_pattern || filter->filter_filename) &&
+	    filter->filter_content)
 		return -EINVAL;
 
 	list_add_tail(&filter->list, &of_overlay_filters);
@@ -653,7 +656,7 @@ static bool of_overlay_filter_pattern(struct of_overlay_filter *f,
 
 static struct of_overlay_filter of_overlay_pattern_filter = {
 	.name = "pattern",
-	.filter_filename = of_overlay_filter_pattern,
+	.filter_pattern = of_overlay_filter_pattern,
 };
 
 static bool of_overlay_filter_filename(struct of_overlay_filter *f,
@@ -665,7 +668,7 @@ static bool of_overlay_filter_filename(struct of_overlay_filter *f,
 
 static struct of_overlay_filter of_overlay_filepattern_filter = {
 	.name = "filepattern",
-	.filter_filename = of_overlay_filter_filename,
+	.filter_pattern = of_overlay_filter_filename,
 };
 
 /**
diff --git a/include/of.h b/include/of.h
index 2258cd501b727797ac00fc4cce1a6fdcfc529d44..1c7c0c867700fcf1aa2316ed1e1702b93aa13011 100644
--- a/include/of.h
+++ b/include/of.h
@@ -1384,7 +1384,8 @@ static inline struct device_node *of_dup_root_node_for_boot(void)
 }
 
 struct of_overlay_filter {
-	bool (*filter_filename)(struct of_overlay_filter *, const char *filename);
+	bool (*filter_filename)(struct of_overlay_filter *, const char *filename); /* deprecated */
+	bool (*filter_pattern)(struct of_overlay_filter *, const char *pattern);
 	bool (*filter_content)(struct of_overlay_filter *, struct device_node *);
 	const char *name;
 	struct list_head list;

-- 
2.39.5

[PATCH v5 09/11] FIT: fit_open: make filename handling more robust

[Marco Felsch]

Require the filename to start at the root '/' directory and resolve any
possible link to make the filename handling more robust.

This is in preparation of adding cached fit_open support.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/image-fit.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index 0067f46e60bc954b418aef3398e2c10856b41c02..1cb407d4d86cb3d0a643149bb08c46caadcd56fe 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -972,12 +972,25 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
  *
  * Return: A handle to a FIT image or a ERR_PTR
  */
-struct fit_handle *fit_open(const char *filename, bool verbose,
+struct fit_handle *fit_open(const char *_filename, bool verbose,
 			    enum bootm_verify verify, loff_t max_size)
 {
 	struct fit_handle *handle;
+	char *filename;
 	int ret;
 
+	if (*_filename != '/') {
+		pr_err("The FIT filename must start with '/'\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* dirfd is ignored, since _filename is absolute */
+	filename = canonicalize_path(AT_FDCWD, _filename);
+	if (!filename) {
+		pr_err("Failed to resolve %s with %s\n", _filename, strerror(errno));
+		return ERR_PTR(-errno);
+	}
+
 	handle = xzalloc(sizeof(struct fit_handle));
 
 	handle->verbose = verbose;
@@ -988,9 +1001,12 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
 	if (ret && ret != -EFBIG) {
 		pr_err("unable to read %s: %pe\n", filename, ERR_PTR(ret));
 		free(handle);
+		free(filename);
 		return ERR_PTR(ret);
 	}
 
+	free(filename);
+
 	handle->fit = handle->fit_alloc;
 
 	ret = fit_do_open(handle);

-- 
2.39.5

[PATCH v5 10/11] FIT: fit_open: save the filename

[Marco Felsch]

This is in preparation of buffering fit_open() calls to not load the
same fit twice if it is still open.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/image-fit.c  | 5 +++--
 include/image-fit.h | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index 1cb407d4d86cb3d0a643149bb08c46caadcd56fe..e4b0a8bd5d1a7283239556dd4532eccd5333e90a 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1005,9 +1005,8 @@ struct fit_handle *fit_open(const char *_filename, bool verbose,
 		return ERR_PTR(ret);
 	}
 
-	free(filename);
-
 	handle->fit = handle->fit_alloc;
+	handle->filename = filename;
 
 	ret = fit_do_open(handle);
 	if (ret) {
@@ -1022,6 +1021,8 @@ static void __fit_close(struct fit_handle *handle)
 {
 	if (handle->root)
 		of_delete_node(handle->root);
+
+	free(handle->filename);
 	free(handle->fit_alloc);
 }
 
diff --git a/include/image-fit.h b/include/image-fit.h
index f8321aa87e11fcea642593e790a37df72a2e93b1..68f70f4365cb7a650596263086f7de2209d5957e 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -13,6 +13,7 @@ struct fit_handle {
 	const void *fit;
 	void *fit_alloc;
 	size_t size;
+	char *filename;
 
 	bool verbose;
 	enum bootm_verify verify;

-- 
2.39.5

[PATCH v5 11/11] FIT: add support to cache opened fit images

[Marco Felsch]

Cache the FIT image fit_open() calls to avoid loading the same FIT image
twice. This is very useful if the same FIT image is used to provide the
base devicetree, kernel and initrd as well as devicetree overlays.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/image-fit.c  | 32 ++++++++++++++++++++++++++++++++
 include/image-fit.h |  4 ++++
 2 files changed, 36 insertions(+)

diff --git a/common/image-fit.c b/common/image-fit.c
index e4b0a8bd5d1a7283239556dd4532eccd5333e90a..23fcbfee3a5ab6c9deec67b4297da986521c114a 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -16,6 +16,7 @@
 #include <fs.h>
 #include <malloc.h>
 #include <linux/ctype.h>
+#include <linux/refcount.h>
 #include <asm/byteorder.h>
 #include <errno.h>
 #include <linux/err.h>
@@ -33,6 +34,8 @@
 #define CHECK_LEVEL_SIG 2
 #define CHECK_LEVEL_MAX 3
 
+static LIST_HEAD(open_fits);
+
 static uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, int size)
 {
 	dt += size;
@@ -902,6 +905,18 @@ void *fit_open_configuration(struct fit_handle *handle, const char *name,
 	return conf_node;
 }
 
+static struct fit_handle *fit_get_handle(const char *filename)
+{
+	struct fit_handle *handle;
+
+	list_for_each_entry(handle, &open_fits, entry) {
+		if (!strcmp(filename, handle->filename))
+			return handle;
+	}
+
+	return NULL;
+}
+
 static int fit_do_open(struct fit_handle *handle)
 {
 	const char *desc = "(no description)";
@@ -951,6 +966,8 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
 	handle->size = size;
 	handle->verify = verify;
 
+	refcount_set(&handle->users, 1);
+
 	ret = fit_do_open(handle);
 	if (ret) {
 		fit_close(handle);
@@ -991,6 +1008,12 @@ struct fit_handle *fit_open(const char *_filename, bool verbose,
 		return ERR_PTR(-errno);
 	}
 
+	handle = fit_get_handle(filename);
+	if (handle) {
+		refcount_inc(&handle->users);
+		return handle;
+	}
+
 	handle = xzalloc(sizeof(struct fit_handle));
 
 	handle->verbose = verbose;
@@ -1008,6 +1031,9 @@ struct fit_handle *fit_open(const char *_filename, bool verbose,
 	handle->fit = handle->fit_alloc;
 	handle->filename = filename;
 
+	refcount_set(&handle->users, 1);
+	list_add(&handle->entry, &open_fits);
+
 	ret = fit_do_open(handle);
 	if (ret) {
 		fit_close(handle);
@@ -1019,9 +1045,15 @@ struct fit_handle *fit_open(const char *_filename, bool verbose,
 
 static void __fit_close(struct fit_handle *handle)
 {
+	if (!refcount_dec_and_test(&handle->users))
+		return;
+
 	if (handle->root)
 		of_delete_node(handle->root);
 
+	if (handle->filename)
+		list_del(&handle->entry);
+
 	free(handle->filename);
 	free(handle->fit_alloc);
 }
diff --git a/include/image-fit.h b/include/image-fit.h
index 68f70f4365cb7a650596263086f7de2209d5957e..f9791ff251c554eda56bf3af98c8abceb056a176 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -7,6 +7,7 @@
 #define __IMAGE_FIT_H__
 
 #include <linux/types.h>
+#include <linux/refcount.h>
 #include <bootm.h>
 
 struct fit_handle {
@@ -15,6 +16,9 @@ struct fit_handle {
 	size_t size;
 	char *filename;
 
+	struct list_head entry;
+	refcount_t users;
+
 	bool verbose;
 	enum bootm_verify verify;
 

-- 
2.39.5

Re: [PATCH v5 07/11] of: overlay: add FIT image overlay support

[Sascha Hauer]

On Mon, Aug 18, 2025 at 07:26:15PM +0200, Marco Felsch wrote:
> This adds the support to load devicetree overlays from a FIT image.
> There are a few options to handle FIT overlays since the FIT overlay
> spec is not very strict.
> 
> This implements the most configurable case where each overlay does have
> its own config node (including the optional signature).
> 
> - The "pattern" filter matches the config-node names (the node names
>   below the configurations node), not the overlay image names (the node
>   names below the images node).
> 
> - The "compatible" filter check doesn't differ from the file based overlay
>   handling.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  Documentation/user/devicetree.rst |  19 +++---
>  drivers/of/overlay.c              | 126 +++++++++++++++++++++++++++++++++++---
>  2 files changed, 128 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/user/devicetree.rst b/Documentation/user/devicetree.rst
> index 30a15a8865c154f8afe23d641da10d0afa84423b..644fb551adcf913ca9ed2ab3f1d5fbe7c367bdf6 100644
> --- a/Documentation/user/devicetree.rst
> +++ b/Documentation/user/devicetree.rst
> @@ -109,20 +109,23 @@ Overlays can be applied to the kernel device tree before it is handed over to
>  the kernel. The behaviour is controlled by different variables:
>  
>  ``global.of.overlay.path``
> -  Overlays are read from this directory. barebox will try to apply all overlays
> -  found here if not limited by one of the other variables below. When the path
> -  given here is an absolute path it is used as is. A relative path is relative
> -  to ``/`` or relative to the rootfs when using bootloader spec.
> +  Overlays are read from this path. The path can either be a directory which
> +  contains the overlays or an absolute path to a FIT-image. barebox will try to

is "absolute path" correct here?

Before it was either an absolute path or a path relative to the
rootfs...

> +  apply all overlays found if not limited by one of the other variables below.
> +  When the path given here is an absolute path it is used as is. A relative
> +  path is relative to ``/`` or relative to the rootfs when using bootloader
> +  spec.

...and this still seems to be the case.

Isn't this true for overlays in FIT images as weel? If yes I suggest to
just remove the "absolute".

>  ``global.of.overlay.compatible``
>    This is a space separated list of compatibles. Only overlays matching one of
>    these compatibles will be applied. When this list is empty then all overlays
>    will be applied. Overlays that don't have a compatible are considered being
>    always compatible.
>  ``global.of.overlay.pattern``
> -  This is a space separated list of file patterns. An overlay is only applied
> -  when its filename matches one of the patterns. The patterns can contain
> -  ``*`` and ``?`` as wildcards. The default is ``*`` which means all files are
> -  applied.
> +  This is a space separated list of file patterns or FIT-image config-node name
> +  patterns. An overlay is only applied when its filename or FIT-image
> +  config-node name matches one of the patterns. The patterns can contain ``*``
> +  and ``?`` as wildcards. The default is ``*`` which means all files or FIT-Image
> +  config-nodes are applied.
>  ``global.of.overlay.filter``
>    This is a space separated list of filters to apply. There are two generic filters:
>    ``pattern`` matches ``global.of.overlay.pattern`` above, ``compatible`` matches
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index f02a12d44f0d53db3fb7bb065461c0ef193d0ab3..6defd6fa6908ff2b2fd00d161464af37ac17e040 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -8,10 +8,12 @@
>   */
>  #define pr_fmt(fmt) "of_overlay: " fmt
>  
> +#include <bootm.h>
>  #include <common.h>
>  #include <of.h>
>  #include <errno.h>
>  #include <globalvar.h>
> +#include <image-fit.h>
>  #include <magicvar.h>
>  #include <string.h>
>  #include <libfile.h>
> @@ -463,8 +465,100 @@ static int of_overlay_apply_dir(struct device_node *root, const char *dirname,
>  	return ret;
>  }
>  
> +static int of_overlay_apply_fit(struct device_node *root, struct fit_handle *fit,
> +				struct device_node *config)
> +{
> +	const char *name = config->name;
> +	struct device_node *overlay;
> +	unsigned long ovl_sz;
> +	const void *ovl;
> +	int ret;
> +
> +	if (!of_overlay_matches_filter(name, NULL))
> +		return 0;

Can we have a
	pr_debug("FIT config \"%s\" doesn't match filter \"%s\"\n",
		name, of_overlay_filter);

Here?

> +
> +	ret = fit_open_image(fit, config, "fdt", &ovl, &ovl_sz);
> +	if (ret)
> +		return ret;
> +
> +	overlay = of_unflatten_dtb(ovl, ovl_sz);

	if (IS_ERR(overlay))
		...

> +
> +	if (!of_overlay_matches_filter(NULL, overlay)) {
> +		ret = 0;
> +		goto out;
> +	}

I think a pr_debug printing the reason would be appropriate here as
well.

> +
> +	ret = of_overlay_apply_tree(root, overlay);
> +	if (ret == -ENODEV)
> +		pr_debug("Not applied %s (not compatible)\n", name);
> +	else if (ret)
> +		pr_err("Cannot apply %s: %s\n", name, strerror(-ret));
> +	else
> +		pr_info("Applied %s\n", name);
> +
> +out:
> +	of_delete_node(overlay);
> +
> +	return ret;
> +}
> +
> +static bool of_overlay_valid_config(struct fit_handle *fit,
> +				    struct device_node *config)
> +{
> +	/*
> +	 * Either kernel or firmware is marked as mandatory by U-Boot
> +	 * (doc/usage/fit/source_file_format.rst) except for overlays
> +	 * (doc/usage/fit/overlay-fdt-boot.rst). Therefore we need to ensure
> +	 * that only "fdt" config nodes are recognized as overlay config node.
> +	 */
> +	if (!fit_has_image(fit, config, "fdt") ||
> +	    fit_has_image(fit, config, "kernel") ||
> +	    fit_has_image(fit, config, "firmware"))
> +		return false;
> +
> +	return true;
> +}
> +
> +static int of_overlay_global_fixup_fit(struct device_node *root,
> +				       const char *fit_path, loff_t fit_size)
> +{
> +	enum bootm_verify verify = bootm_get_verify_mode();
> +	struct device_node *conf_node;
> +	struct fit_handle *fit;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_FITIMAGE)) {
> +		pr_err("FIT based overlay handling requested while CONFIG_FITIMAGE=n\n");
> +		return -EINVAL;
> +	}
> +
> +	fit = fit_open(fit_path, 0, verify, fit_size);
> +	if (IS_ERR(fit)) {
> +		pr_err("Loading FIT image %s failed with: %pe\n", fit_path, fit);
> +		return PTR_ERR(fit);
> +	}
> +
> +	for_each_child_of_node(fit->configurations, conf_node) {
> +		if (!of_overlay_valid_config(fit, conf_node))
> +			continue;
> +
> +		ret = fit_config_verify_signature(fit, conf_node);
> +		if (ret)
> +			goto out;
> +
> +		ret = of_overlay_apply_fit(root, fit, conf_node);
> +		if (ret)
> +			goto out;
> +	}
> +
> +out:
> +	fit_close(fit);
> +	return ret;
> +}
> +
>  static int of_overlay_global_fixup(struct device_node *root, void *data)
>  {
> +	struct stat s;
>  	char *dir;
>  	int ret;
>  
> @@ -476,10 +570,24 @@ static int of_overlay_global_fixup(struct device_node *root, void *data)
>  	else
>  		dir = concat_path_file(of_overlay_basedir, of_overlay_path);
>  
> -	ret = of_overlay_apply_dir(root, dir, true);
>  
> -	free(dir);
> +	if (stat(dir, &s)) {
> +		pr_err("Cannot stat global.of.overlay.path (%s): %s\n",
> +		       dir, strerror(errno));

Use %m which prints strerror(errno) directly.

Sascha

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

Re: [PATCH v5 09/11] FIT: fit_open: make filename handling more robust

[Sascha Hauer]

On Mon, Aug 18, 2025 at 07:26:17PM +0200, Marco Felsch wrote:
> Require the filename to start at the root '/' directory and resolve any
> possible link to make the filename handling more robust.
> 
> This is in preparation of adding cached fit_open support.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  common/image-fit.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 0067f46e60bc954b418aef3398e2c10856b41c02..1cb407d4d86cb3d0a643149bb08c46caadcd56fe 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -972,12 +972,25 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
>   *
>   * Return: A handle to a FIT image or a ERR_PTR
>   */
> -struct fit_handle *fit_open(const char *filename, bool verbose,
> +struct fit_handle *fit_open(const char *_filename, bool verbose,
>  			    enum bootm_verify verify, loff_t max_size)
>  {
>  	struct fit_handle *handle;
> +	char *filename;
>  	int ret;
>  
> +	if (*_filename != '/') {
> +		pr_err("The FIT filename must start with '/'\n");
> +		return ERR_PTR(-EINVAL);
> +	}

Drop this. We want to be able to call fit_open() with relative pathes...

> +
> +	/* dirfd is ignored, since _filename is absolute */
> +	filename = canonicalize_path(AT_FDCWD, _filename);

...and you are calling canonicalize_path() anyway which gives you an
absolute path from the potentially relative input path.

> +	if (!filename) {
> +		pr_err("Failed to resolve %s with %s\n", _filename, strerror(errno));

		pr_err("Cannot open %s: %m\n", filename);

I think the "Failed to resolve" is misleading here.

Sascha

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

Re: [PATCH v5 07/11] of: overlay: add FIT image overlay support

[Marco Felsch]

On 25-08-19, Sascha Hauer wrote:
> On Mon, Aug 18, 2025 at 07:26:15PM +0200, Marco Felsch wrote:
> > This adds the support to load devicetree overlays from a FIT image.
> > There are a few options to handle FIT overlays since the FIT overlay
> > spec is not very strict.
> > 
> > This implements the most configurable case where each overlay does have
> > its own config node (including the optional signature).
> > 
> > - The "pattern" filter matches the config-node names (the node names
> >   below the configurations node), not the overlay image names (the node
> >   names below the images node).
> > 
> > - The "compatible" filter check doesn't differ from the file based overlay
> >   handling.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  Documentation/user/devicetree.rst |  19 +++---
> >  drivers/of/overlay.c              | 126 +++++++++++++++++++++++++++++++++++---
> >  2 files changed, 128 insertions(+), 17 deletions(-)
> > 
> > diff --git a/Documentation/user/devicetree.rst b/Documentation/user/devicetree.rst
> > index 30a15a8865c154f8afe23d641da10d0afa84423b..644fb551adcf913ca9ed2ab3f1d5fbe7c367bdf6 100644
> > --- a/Documentation/user/devicetree.rst
> > +++ b/Documentation/user/devicetree.rst
> > @@ -109,20 +109,23 @@ Overlays can be applied to the kernel device tree before it is handed over to
> >  the kernel. The behaviour is controlled by different variables:
> >  
> >  ``global.of.overlay.path``
> > -  Overlays are read from this directory. barebox will try to apply all overlays
> > -  found here if not limited by one of the other variables below. When the path
> > -  given here is an absolute path it is used as is. A relative path is relative
> > -  to ``/`` or relative to the rootfs when using bootloader spec.
> > +  Overlays are read from this path. The path can either be a directory which
> > +  contains the overlays or an absolute path to a FIT-image. barebox will try to
> 
> is "absolute path" correct here?
> 
> Before it was either an absolute path or a path relative to the
> rootfs...

IMHO the only reason for using FIT as overlay supplier is because of the
fact the the system implements a verified boot flow. For such use-case I
wouldn't allow relative paths.

> 
> > +  apply all overlays found if not limited by one of the other variables below.
> > +  When the path given here is an absolute path it is used as is. A relative
> > +  path is relative to ``/`` or relative to the rootfs when using bootloader
> > +  spec.
> 
> ...and this still seems to be the case.

Of course, we need to handle both cases but I would mention that the FIT
image path shall be absolute albeit the implementation supports both.

> Isn't this true for overlays in FIT images as weel? If yes I suggest to
> just remove the "absolute".
> 
> >  ``global.of.overlay.compatible``
> >    This is a space separated list of compatibles. Only overlays matching one of
> >    these compatibles will be applied. When this list is empty then all overlays
> >    will be applied. Overlays that don't have a compatible are considered being
> >    always compatible.
> >  ``global.of.overlay.pattern``
> > -  This is a space separated list of file patterns. An overlay is only applied
> > -  when its filename matches one of the patterns. The patterns can contain
> > -  ``*`` and ``?`` as wildcards. The default is ``*`` which means all files are
> > -  applied.
> > +  This is a space separated list of file patterns or FIT-image config-node name
> > +  patterns. An overlay is only applied when its filename or FIT-image
> > +  config-node name matches one of the patterns. The patterns can contain ``*``
> > +  and ``?`` as wildcards. The default is ``*`` which means all files or FIT-Image
> > +  config-nodes are applied.
> >  ``global.of.overlay.filter``
> >    This is a space separated list of filters to apply. There are two generic filters:
> >    ``pattern`` matches ``global.of.overlay.pattern`` above, ``compatible`` matches
> > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> > index f02a12d44f0d53db3fb7bb065461c0ef193d0ab3..6defd6fa6908ff2b2fd00d161464af37ac17e040 100644
> > --- a/drivers/of/overlay.c
> > +++ b/drivers/of/overlay.c
> > @@ -8,10 +8,12 @@
> >   */
> >  #define pr_fmt(fmt) "of_overlay: " fmt
> >  
> > +#include <bootm.h>
> >  #include <common.h>
> >  #include <of.h>
> >  #include <errno.h>
> >  #include <globalvar.h>
> > +#include <image-fit.h>
> >  #include <magicvar.h>
> >  #include <string.h>
> >  #include <libfile.h>
> > @@ -463,8 +465,100 @@ static int of_overlay_apply_dir(struct device_node *root, const char *dirname,
> >  	return ret;
> >  }
> >  
> > +static int of_overlay_apply_fit(struct device_node *root, struct fit_handle *fit,
> > +				struct device_node *config)
> > +{
> > +	const char *name = config->name;
> > +	struct device_node *overlay;
> > +	unsigned long ovl_sz;
> > +	const void *ovl;
> > +	int ret;
> > +
> > +	if (!of_overlay_matches_filter(name, NULL))
> > +		return 0;
> 
> Can we have a
> 	pr_debug("FIT config \"%s\" doesn't match filter \"%s\"\n",
> 		name, of_overlay_filter);
> 
> Here?

Sure.

> > +
> > +	ret = fit_open_image(fit, config, "fdt", &ovl, &ovl_sz);
> > +	if (ret)
> > +		return ret;
> > +
> > +	overlay = of_unflatten_dtb(ovl, ovl_sz);
> 
> 	if (IS_ERR(overlay))
> 		...

Sure.

> > +	if (!of_overlay_matches_filter(NULL, overlay)) {
> > +		ret = 0;
> > +		goto out;
> > +	}
> 
> I think a pr_debug printing the reason would be appropriate here as
> well.

Sure.

> > +
> > +	ret = of_overlay_apply_tree(root, overlay);
> > +	if (ret == -ENODEV)
> > +		pr_debug("Not applied %s (not compatible)\n", name);
> > +	else if (ret)
> > +		pr_err("Cannot apply %s: %s\n", name, strerror(-ret));
> > +	else
> > +		pr_info("Applied %s\n", name);
> > +
> > +out:
> > +	of_delete_node(overlay);
> > +
> > +	return ret;
> > +}
> > +
> > +static bool of_overlay_valid_config(struct fit_handle *fit,
> > +				    struct device_node *config)
> > +{
> > +	/*
> > +	 * Either kernel or firmware is marked as mandatory by U-Boot
> > +	 * (doc/usage/fit/source_file_format.rst) except for overlays
> > +	 * (doc/usage/fit/overlay-fdt-boot.rst). Therefore we need to ensure
> > +	 * that only "fdt" config nodes are recognized as overlay config node.
> > +	 */
> > +	if (!fit_has_image(fit, config, "fdt") ||
> > +	    fit_has_image(fit, config, "kernel") ||
> > +	    fit_has_image(fit, config, "firmware"))
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +static int of_overlay_global_fixup_fit(struct device_node *root,
> > +				       const char *fit_path, loff_t fit_size)
> > +{
> > +	enum bootm_verify verify = bootm_get_verify_mode();
> > +	struct device_node *conf_node;
> > +	struct fit_handle *fit;
> > +	int ret;
> > +
> > +	if (!IS_ENABLED(CONFIG_FITIMAGE)) {
> > +		pr_err("FIT based overlay handling requested while CONFIG_FITIMAGE=n\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	fit = fit_open(fit_path, 0, verify, fit_size);
> > +	if (IS_ERR(fit)) {
> > +		pr_err("Loading FIT image %s failed with: %pe\n", fit_path, fit);
> > +		return PTR_ERR(fit);
> > +	}
> > +
> > +	for_each_child_of_node(fit->configurations, conf_node) {
> > +		if (!of_overlay_valid_config(fit, conf_node))
> > +			continue;
> > +
> > +		ret = fit_config_verify_signature(fit, conf_node);
> > +		if (ret)
> > +			goto out;
> > +
> > +		ret = of_overlay_apply_fit(root, fit, conf_node);
> > +		if (ret)
> > +			goto out;
> > +	}
> > +
> > +out:
> > +	fit_close(fit);
> > +	return ret;
> > +}
> > +
> >  static int of_overlay_global_fixup(struct device_node *root, void *data)
> >  {
> > +	struct stat s;
> >  	char *dir;
> >  	int ret;
> >  
> > @@ -476,10 +570,24 @@ static int of_overlay_global_fixup(struct device_node *root, void *data)
> >  	else
> >  		dir = concat_path_file(of_overlay_basedir, of_overlay_path);
> >  
> > -	ret = of_overlay_apply_dir(root, dir, true);
> >  
> > -	free(dir);
> > +	if (stat(dir, &s)) {
> > +		pr_err("Cannot stat global.of.overlay.path (%s): %s\n",
> > +		       dir, strerror(errno));
> 
> Use %m which prints strerror(errno) directly.

Of course, thanks.

Regards,
  Marco

> 
> Sascha
> 
> -- 
> 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 |
> 

Re: [PATCH v5 09/11] FIT: fit_open: make filename handling more robust

[Marco Felsch]

On 25-08-19, Sascha Hauer wrote:
> On Mon, Aug 18, 2025 at 07:26:17PM +0200, Marco Felsch wrote:
> > Require the filename to start at the root '/' directory and resolve any
> > possible link to make the filename handling more robust.
> > 
> > This is in preparation of adding cached fit_open support.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  common/image-fit.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/common/image-fit.c b/common/image-fit.c
> > index 0067f46e60bc954b418aef3398e2c10856b41c02..1cb407d4d86cb3d0a643149bb08c46caadcd56fe 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -972,12 +972,25 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
> >   *
> >   * Return: A handle to a FIT image or a ERR_PTR
> >   */
> > -struct fit_handle *fit_open(const char *filename, bool verbose,
> > +struct fit_handle *fit_open(const char *_filename, bool verbose,
> >  			    enum bootm_verify verify, loff_t max_size)
> >  {
> >  	struct fit_handle *handle;
> > +	char *filename;
> >  	int ret;
> >  
> > +	if (*_filename != '/') {
> > +		pr_err("The FIT filename must start with '/'\n");
> > +		return ERR_PTR(-EINVAL);
> > +	}
> 
> Drop this. We want to be able to call fit_open() with relative pathes...

IMHO FIT containers are only used in verified-boot setups. I wouldn't
allow relative paths in such use-cases.

> 
> > +
> > +	/* dirfd is ignored, since _filename is absolute */
> > +	filename = canonicalize_path(AT_FDCWD, _filename);
> 
> ...and you are calling canonicalize_path() anyway which gives you an
> absolute path from the potentially relative input path.

But we don't know the base directory to which this path would relative
too.

> > +	if (!filename) {
> > +		pr_err("Failed to resolve %s with %s\n", _filename, strerror(errno));
> 
> 		pr_err("Cannot open %s: %m\n", filename);
> 
> I think the "Failed to resolve" is misleading here.

You're right, I will change this.

Regards,
  Marco

> 
> Sascha
> 
> -- 
> 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 |
> 

Re: [PATCH v5 09/11] FIT: fit_open: make filename handling more robust

[Sascha Hauer]

On Tue, Aug 19, 2025 at 10:19:41AM +0200, Marco Felsch wrote:
> On 25-08-19, Sascha Hauer wrote:
> > On Mon, Aug 18, 2025 at 07:26:17PM +0200, Marco Felsch wrote:
> > > Require the filename to start at the root '/' directory and resolve any
> > > possible link to make the filename handling more robust.
> > > 
> > > This is in preparation of adding cached fit_open support.
> > > 
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > ---
> > >  common/image-fit.c | 18 +++++++++++++++++-
> > >  1 file changed, 17 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/common/image-fit.c b/common/image-fit.c
> > > index 0067f46e60bc954b418aef3398e2c10856b41c02..1cb407d4d86cb3d0a643149bb08c46caadcd56fe 100644
> > > --- a/common/image-fit.c
> > > +++ b/common/image-fit.c
> > > @@ -972,12 +972,25 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
> > >   *
> > >   * Return: A handle to a FIT image or a ERR_PTR
> > >   */
> > > -struct fit_handle *fit_open(const char *filename, bool verbose,
> > > +struct fit_handle *fit_open(const char *_filename, bool verbose,
> > >  			    enum bootm_verify verify, loff_t max_size)
> > >  {
> > >  	struct fit_handle *handle;
> > > +	char *filename;
> > >  	int ret;
> > >  
> > > +	if (*_filename != '/') {
> > > +		pr_err("The FIT filename must start with '/'\n");
> > > +		return ERR_PTR(-EINVAL);
> > > +	}
> > 
> > Drop this. We want to be able to call fit_open() with relative pathes...
> 
> IMHO FIT containers are only used in verified-boot setups. I wouldn't
> allow relative paths in such use-cases.

That's your/our usecase, but you don't know what others are doing and a
"bootm foo.fit" should just work without complaining about a relative
path.

> 
> > 
> > > +
> > > +	/* dirfd is ignored, since _filename is absolute */
> > > +	filename = canonicalize_path(AT_FDCWD, _filename);
> > 
> > ...and you are calling canonicalize_path() anyway which gives you an
> > absolute path from the potentially relative input path.
> 
> But we don't know the base directory to which this path would relative
> too.

canonicalize_path() returns an absolute path.

Sascha

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

Re: [PATCH v5 09/11] FIT: fit_open: make filename handling more robust

[Marco Felsch]

On 25-08-19, Sascha Hauer wrote:
> On Tue, Aug 19, 2025 at 10:19:41AM +0200, Marco Felsch wrote:
> > On 25-08-19, Sascha Hauer wrote:
> > > On Mon, Aug 18, 2025 at 07:26:17PM +0200, Marco Felsch wrote:
> > > > Require the filename to start at the root '/' directory and resolve any
> > > > possible link to make the filename handling more robust.
> > > > 
> > > > This is in preparation of adding cached fit_open support.
> > > > 
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > ---
> > > >  common/image-fit.c | 18 +++++++++++++++++-
> > > >  1 file changed, 17 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/common/image-fit.c b/common/image-fit.c
> > > > index 0067f46e60bc954b418aef3398e2c10856b41c02..1cb407d4d86cb3d0a643149bb08c46caadcd56fe 100644
> > > > --- a/common/image-fit.c
> > > > +++ b/common/image-fit.c
> > > > @@ -972,12 +972,25 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
> > > >   *
> > > >   * Return: A handle to a FIT image or a ERR_PTR
> > > >   */
> > > > -struct fit_handle *fit_open(const char *filename, bool verbose,
> > > > +struct fit_handle *fit_open(const char *_filename, bool verbose,
> > > >  			    enum bootm_verify verify, loff_t max_size)
> > > >  {
> > > >  	struct fit_handle *handle;
> > > > +	char *filename;
> > > >  	int ret;
> > > >  
> > > > +	if (*_filename != '/') {
> > > > +		pr_err("The FIT filename must start with '/'\n");
> > > > +		return ERR_PTR(-EINVAL);
> > > > +	}
> > > 
> > > Drop this. We want to be able to call fit_open() with relative pathes...
> > 
> > IMHO FIT containers are only used in verified-boot setups. I wouldn't
> > allow relative paths in such use-cases.
> 
> That's your/our usecase, but you don't know what others are doing and a
> "bootm foo.fit" should just work without complaining about a relative
> path.

Okay, albeit I'm still not convinced that 'bootm foo.fit' is a good
idea. But you're right, enforcing an absolute path may be a bit too
strict.

> > > > +
> > > > +	/* dirfd is ignored, since _filename is absolute */
> > > > +	filename = canonicalize_path(AT_FDCWD, _filename);
> > > 
> > > ...and you are calling canonicalize_path() anyway which gives you an
> > > absolute path from the potentially relative input path.
> > 
> > But we don't know the base directory to which this path would relative
> > too.
> 
> canonicalize_path() returns an absolute path.

I know and it does the job to resolve the whole path to use it for
caching purpose. But as said, I just using the CWD may trigger
unintentionally behaviors. Anyway, I will drop the '/' root restriction.

Regards,
  Marco

> 
> Sascha
> 
> -- 
> 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 |
> 

Re: [PATCH v5 07/11] of: overlay: add FIT image overlay support

[Marco Felsch]

Hi Sascha,

On 25-08-19, Sascha Hauer wrote:
> On Mon, Aug 18, 2025 at 07:26:15PM +0200, Marco Felsch wrote:

...

> > +static int of_overlay_apply_fit(struct device_node *root, struct fit_handle *fit,
> > +				struct device_node *config)
> > +{
> > +	const char *name = config->name;
> > +	struct device_node *overlay;
> > +	unsigned long ovl_sz;
> > +	const void *ovl;
> > +	int ret;
> > +
> > +	if (!of_overlay_matches_filter(name, NULL))
> > +		return 0;
> 
> Can we have a
> 	pr_debug("FIT config \"%s\" doesn't match filter \"%s\"\n",
> 		name, of_overlay_filter);
> 
> Here?
> 
> > +
> > +	ret = fit_open_image(fit, config, "fdt", &ovl, &ovl_sz);
> > +	if (ret)
> > +		return ret;
> > +
> > +	overlay = of_unflatten_dtb(ovl, ovl_sz);
> 
> 	if (IS_ERR(overlay))
> 		...
> 
> > +
> > +	if (!of_overlay_matches_filter(NULL, overlay)) {
> > +		ret = 0;
> > +		goto out;
> > +	}
> 
> I think a pr_debug printing the reason would be appropriate here as
> well.

Instead of adding it here I adapted the three cases in
of_overlay_matches_filter() which didn't had a pr_debug() yet to cover
all possible cases.

I also added pr_debug("Process FIT config \"%s\"\n", name); at the
beginning of of_overlay_apply_fit().

Regards,
  Marco