From 646be6d5cd37dc9f1e79f4c6677b872932605a2b Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Fri, 8 May 2026 21:31:59 +0000 Subject: boot/fit: read default-config property from the configurations node In fit_print_contents() the default configuration's unit name is read by calling fdt_getprop() with noffset rather than confs_noffset. Today this happens to work by coincidence: the preceding loop walks /images using fdt_next_node(), and when iteration leaves the subtree libfdt returns the offset of the next sibling in DFS order, which by FIT layout convention is /configurations. The depth counter then drops below zero and the loop exits with noffset still pointing at /configurations. This relies on /images and /configurations being adjacent siblings and on the implementation detail of fdt_next_node()'s post-exhaustion return value. It also blocks a follow-up conversion to fdt_for_each_subnode(), whose post-loop loop variable is a negative error code rather than a valid offset. Use confs_noffset directly, which the comment immediately above the call already names as the source. Signed-off-by: Aristo Chen Reviewed-by: Simon Glass --- boot/image-fit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boot/image-fit.c b/boot/image-fit.c index 2d2709aa5b1..5a502e93106 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -449,7 +449,7 @@ void fit_print_contents(const void *fit) } /* get default configuration unit name from default property */ - uname = (char *)fdt_getprop(fit, noffset, FIT_DEFAULT_PROP, NULL); + uname = (char *)fdt_getprop(fit, confs_noffset, FIT_DEFAULT_PROP, NULL); if (uname) printf("%s Default Configuration: '%s'\n", p, uname); -- cgit v1.3.1 From b31f551bfcf05cec628fc79af5d12a601e0a1f48 Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Fri, 8 May 2026 21:32:00 +0000 Subject: test: fit: regression test for default-config print with reversed node order Add a test that builds a FIT whose /configurations node is defined before /images in the source, runs iminfo, and asserts that the "Default Configuration: ''" line appears in the output. Before the fix in the preceding commit ("boot/fit: read default-config property from the configurations node"), fit_print_contents() read the default-config property using the loop variable left over from iterating /images children. With /images defined first that variable accidentally pointed at /configurations and the line printed correctly; with /configurations defined first the read returned NULL and the line was silently omitted. The new test exercises the latter layout, which had no coverage. iminfo and the fit_print_contents() path had no test coverage at all before this commit. Signed-off-by: Aristo Chen Reviewed-by: Simon Glass --- test/py/tests/test_fit.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py index bcaaa6a5fc4..a526307ea7a 100755 --- a/test/py/tests/test_fit.py +++ b/test/py/tests/test_fit.py @@ -438,3 +438,56 @@ class TestFitImage: output = ubman.run_command_list(cmds) assert "can't get kernel image!" in '\n'.join(output) + + def test_fit_iminfo_configs_first(self, ubman, fsetup): + """Regression: iminfo prints "Default Configuration" even when + /configurations is defined before /images in the source. + + fit_print_contents() in boot/image-fit.c used to read the default + configuration name from whatever offset libfdt happened to return + after iterating /images children. With /images defined first that + offset accidentally landed on /configurations; with /configurations + defined first the read returned NULL and the line silently went + missing. Fixed in commit "boot/fit: read default-config property + from the configurations node". + """ + configs_first_its = ''' +/dts-v1/; + +/ { + description = "FIT with /configurations before /images"; + #address-cells = <1>; + + configurations { + default = "conf-1"; + conf-1 { + description = "first config"; + kernel = "kernel-1"; + }; + }; + + images { + kernel-1 { + description = "first image"; + data = /incbin/("%(kernel)s"); + type = "kernel"; + arch = "sandbox"; + os = "linux"; + compression = "none"; + load = <0x40000>; + entry = <0x40000>; + }; + }; +}; +''' + fit = fit_util.make_fit(ubman, fsetup['mkimage'], configs_first_its, + fsetup, basename='configs-first.fit') + cmds = [ + 'host load hostfs 0 %#x %s' % (fsetup['fit_addr'], fit), + 'iminfo %#x' % fsetup['fit_addr'], + ] + output = '\n'.join(ubman.run_command_list(cmds)) + assert "Default Configuration: 'conf-1'" in output, ( + 'iminfo output is missing the "Default Configuration" line for a ' + 'FIT whose /configurations node precedes /images. Output was:\n' + + output) -- cgit v1.3.1 From 2c9b117aa4811d583f2832b37a69f25c761ffc86 Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Fri, 8 May 2026 21:32:01 +0000 Subject: boot/fit: use fdt_for_each_subnode() in image-fit.c Replace the verbose fdt_next_node() + ndepth pattern with the fdt_for_each_subnode() macro at all seven sites in boot/image-fit.c where the loop only ever processes direct children. The macro is already defined in and used in boot/image-fit-sig.c, so this brings image-fit.c in line with the rest of the FIT code. The conversions are equivalence-preserving: - fit_get_subimage_count(): the depth-1 filter and the macro are both restricted to direct children. - fit_conf_print(): the parameter is named noffset, so the loop now uses sub_noffset to keep the parent reference stable. - fit_print_contents(): the count reset that lived inside the for initialiser is moved out as an explicit assignment before each loop, so the second loop still starts from zero. - fit_image_print(): straightforward replacement. - fit_all_image_verify(): same shape as the print loops, with the count reset moved out as an explicit assignment before the loop. - fit_conf_find_compat(): the body's "if (ndepth > 1) continue" guard is redundant once the macro is in use, and is dropped. No behaviour changes outside of these mechanical reductions. Local ndepth declarations that are no longer referenced are removed. Signed-off-by: Aristo Chen Reviewed-by: Simon Glass --- boot/image-fit.c | 107 ++++++++++++++----------------------------------------- 1 file changed, 27 insertions(+), 80 deletions(-) diff --git a/boot/image-fit.c b/boot/image-fit.c index 5a502e93106..dddc0d97928 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -156,18 +156,10 @@ static void fit_get_debug(const void *fit, int noffset, int fit_get_subimage_count(const void *fit, int images_noffset) { int noffset; - int ndepth; int count = 0; - /* Process its subnodes, print out component images details */ - for (ndepth = 0, count = 0, - noffset = fdt_next_node(fit, images_noffset, &ndepth); - (noffset >= 0) && (ndepth > 0); - noffset = fdt_next_node(fit, noffset, &ndepth)) { - if (ndepth == 1) { - count++; - } - } + fdt_for_each_subnode(noffset, fit, images_noffset) + count++; return count; } @@ -291,7 +283,7 @@ static void fit_conf_print(const void *fit, int noffset, const char *p) const char *uname; int ret; int fdt_index, loadables_index; - int ndepth; + int sub_noffset; /* Mandatory properties */ ret = fit_get_desc(fit, noffset, &desc); @@ -357,14 +349,8 @@ static void fit_conf_print(const void *fit, int noffset, const char *p) } /* Process all hash subnodes of the component configuration node */ - for (ndepth = 0, noffset = fdt_next_node(fit, noffset, &ndepth); - (noffset >= 0) && (ndepth > 0); - noffset = fdt_next_node(fit, noffset, &ndepth)) { - if (ndepth == 1) { - /* Direct child node of the component configuration node */ - fit_image_print_verification_data(fit, noffset, p); - } - } + fdt_for_each_subnode(sub_noffset, fit, noffset) + fit_image_print_verification_data(fit, sub_noffset, p); } /** @@ -386,8 +372,7 @@ void fit_print_contents(const void *fit) int images_noffset; int confs_noffset; int noffset; - int ndepth; - int count = 0; + int count; int ret; const char *p; time_t timestamp; @@ -424,20 +409,12 @@ void fit_print_contents(const void *fit) } /* Process its subnodes, print out component images details */ - for (ndepth = 0, count = 0, - noffset = fdt_next_node(fit, images_noffset, &ndepth); - (noffset >= 0) && (ndepth > 0); - noffset = fdt_next_node(fit, noffset, &ndepth)) { - if (ndepth == 1) { - /* - * Direct child node of the images parent node, - * i.e. component image node. - */ - printf("%s Image %u (%s)\n", p, count++, - fit_get_name(fit, noffset, NULL)); + count = 0; + fdt_for_each_subnode(noffset, fit, images_noffset) { + printf("%s Image %u (%s)\n", p, count++, + fit_get_name(fit, noffset, NULL)); - fit_image_print(fit, noffset, p); - } + fit_image_print(fit, noffset, p); } /* Find configurations parent node offset */ @@ -454,20 +431,12 @@ void fit_print_contents(const void *fit) printf("%s Default Configuration: '%s'\n", p, uname); /* Process its subnodes, print out configurations details */ - for (ndepth = 0, count = 0, - noffset = fdt_next_node(fit, confs_noffset, &ndepth); - (noffset >= 0) && (ndepth > 0); - noffset = fdt_next_node(fit, noffset, &ndepth)) { - if (ndepth == 1) { - /* - * Direct child node of the configurations parent node, - * i.e. configuration node. - */ - printf("%s Configuration %u (%s)\n", p, count++, - fit_get_name(fit, noffset, NULL)); + count = 0; + fdt_for_each_subnode(noffset, fit, confs_noffset) { + printf("%s Configuration %u (%s)\n", p, count++, + fit_get_name(fit, noffset, NULL)); - fit_conf_print(fit, noffset, p); - } + fit_conf_print(fit, noffset, p); } } @@ -494,7 +463,6 @@ void fit_image_print(const void *fit, int image_noffset, const char *p) ulong load, entry; const void *data; int noffset; - int ndepth; int ret; if (!CONFIG_IS_ENABLED(FIT_PRINT)) @@ -584,14 +552,8 @@ void fit_image_print(const void *fit, int image_noffset, const char *p) } /* Process all hash subnodes of the component image node */ - for (ndepth = 0, noffset = fdt_next_node(fit, image_noffset, &ndepth); - (noffset >= 0) && (ndepth > 0); - noffset = fdt_next_node(fit, noffset, &ndepth)) { - if (ndepth == 1) { - /* Direct child node of the component image node */ - fit_image_print_verification_data(fit, noffset, p); - } - } + fdt_for_each_subnode(noffset, fit, image_noffset) + fit_image_print_verification_data(fit, noffset, p); } /** @@ -1477,7 +1439,6 @@ int fit_all_image_verify(const void *fit) { int images_noffset; int noffset; - int ndepth; int count; /* Find images parent node offset */ @@ -1491,23 +1452,15 @@ int fit_all_image_verify(const void *fit) /* Process all image subnodes, check hashes for each */ printf("## Checking hash(es) for FIT Image at %08lx ...\n", (ulong)fit); - for (ndepth = 0, count = 0, - noffset = fdt_next_node(fit, images_noffset, &ndepth); - (noffset >= 0) && (ndepth > 0); - noffset = fdt_next_node(fit, noffset, &ndepth)) { - if (ndepth == 1) { - /* - * Direct child node of the images parent node, - * i.e. component image node. - */ - printf(" Hash(es) for Image %u (%s): ", count, - fit_get_name(fit, noffset, NULL)); - count++; + count = 0; + fdt_for_each_subnode(noffset, fit, images_noffset) { + printf(" Hash(es) for Image %u (%s): ", count, + fit_get_name(fit, noffset, NULL)); + count++; - if (!fit_image_verify(fit, noffset)) - return 0; - printf("\n"); - } + if (!fit_image_verify(fit, noffset)) + return 0; + printf("\n"); } return 1; } @@ -1734,7 +1687,6 @@ int fit_check_format(const void *fit, ulong size) int fit_conf_find_compat(const void *fit, const void *fdt) { - int ndepth = 0; int noffset, confs_noffset, images_noffset; const void *fdt_compat; int fdt_compat_len; @@ -1757,9 +1709,7 @@ int fit_conf_find_compat(const void *fit, const void *fdt) /* * Loop over the configurations in the FIT image. */ - for (noffset = fdt_next_node(fit, confs_noffset, &ndepth); - (noffset >= 0) && (ndepth > 0); - noffset = fdt_next_node(fit, noffset, &ndepth)) { + fdt_for_each_subnode(noffset, fit, confs_noffset) { const void *fdt; const char *kfdt_name; int kfdt_noffset, compat_noffset; @@ -1768,9 +1718,6 @@ int fit_conf_find_compat(const void *fit, const void *fdt) size_t sz; int i; - if (ndepth > 1) - continue; - /* If there's a compat property in the config node, use that. */ if (fdt_getprop(fit, noffset, FIT_COMPAT_PROP, NULL)) { fdt = fit; /* search in FIT image */ -- cgit v1.3.1