[PATCH] common/spl: fix endless loop in spl_fit_append_fdt()

[PATCH] common/spl: fix endless loop in spl_fit_append_fdt()

[Michael Walle]

Technically, commit 24bf44cf88e7 ("spl: fit: Do not fail immediately if
an overlay is not available") introduced that regression as the code
will never advance if spl_fit_get_image_name() will return an error. But
at that time, spl_fit_get_image_node() was used in spl_fit_append_fdt()
which calls fdt_subnode_offset() to get the image node. And I presume
the commit was about the latter failing gracefully and trying the next
one.

But with commit b13eaf3bb4e6 ("spl: fit: Add board level function to
decide application of DTO") that behavior changed and the loop in
spl_fit_append_fdt() no longer uses spl_fit_get_image_node() but
spl_fit_get_image_name() directly. Thus it doesn't make any sense to not
break the loop if that fails.

Also, the original use case of commit 24bf44cf88e7 ("spl: fit: Do not
fail immediately if an overlay is not available") is preserved because
spl_subnode_offset() is now called within the loop and errors are
handled gracefully (and advancing the index).

Fixes: b13eaf3bb4e6 ("spl: fit: Add board level function to decide application of DTO")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 common/spl/spl_fit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 746c3d2fa28..342262dd112 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -448,8 +448,8 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
 				debug("%s: No additional FDT node\n", __func__);
 				ret = 0;
 				break;
-			} else if (ret < 0) {
-				continue;
+			} else {
+				break;
 			}
 
 			ret = board_spl_fit_append_fdt_skip(str);
-- 
2.47.3

Re: [PATCH] common/spl: fix endless loop in spl_fit_append_fdt()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1372 bytes --]

On Mon, Oct 27, 2025 at 12:05:21PM +0100, Michael Walle wrote:

> Technically, commit 24bf44cf88e7 ("spl: fit: Do not fail immediately if
> an overlay is not available") introduced that regression as the code
> will never advance if spl_fit_get_image_name() will return an error. But
> at that time, spl_fit_get_image_node() was used in spl_fit_append_fdt()
> which calls fdt_subnode_offset() to get the image node. And I presume
> the commit was about the latter failing gracefully and trying the next
> one.
> 
> But with commit b13eaf3bb4e6 ("spl: fit: Add board level function to
> decide application of DTO") that behavior changed and the loop in
> spl_fit_append_fdt() no longer uses spl_fit_get_image_node() but
> spl_fit_get_image_name() directly. Thus it doesn't make any sense to not
> break the loop if that fails.
> 
> Also, the original use case of commit 24bf44cf88e7 ("spl: fit: Do not
> fail immediately if an overlay is not available") is preserved because
> spl_subnode_offset() is now called within the loop and errors are
> handled gracefully (and advancing the index).
> 
> Fixes: b13eaf3bb4e6 ("spl: fit: Add board level function to decide application of DTO")
> Signed-off-by: Michael Walle <mwalle@kernel.org>

Thanks for explaining what's going on in such detail.

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] common/spl: fix endless loop in spl_fit_append_fdt()

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]

On Tue Oct 28, 2025 at 12:03 AM CET, Tom Rini wrote:
> On Mon, Oct 27, 2025 at 12:05:21PM +0100, Michael Walle wrote:
>
>> Technically, commit 24bf44cf88e7 ("spl: fit: Do not fail immediately if
>> an overlay is not available") introduced that regression as the code
>> will never advance if spl_fit_get_image_name() will return an error. But
>> at that time, spl_fit_get_image_node() was used in spl_fit_append_fdt()
>> which calls fdt_subnode_offset() to get the image node. And I presume
>> the commit was about the latter failing gracefully and trying the next
>> one.
>> 
>> But with commit b13eaf3bb4e6 ("spl: fit: Add board level function to
>> decide application of DTO") that behavior changed and the loop in
>> spl_fit_append_fdt() no longer uses spl_fit_get_image_node() but
>> spl_fit_get_image_name() directly. Thus it doesn't make any sense to not
>> break the loop if that fails.
>> 
>> Also, the original use case of commit 24bf44cf88e7 ("spl: fit: Do not
>> fail immediately if an overlay is not available") is preserved because
>> spl_subnode_offset() is now called within the loop and errors are
>> handled gracefully (and advancing the index).
>> 
>> Fixes: b13eaf3bb4e6 ("spl: fit: Add board level function to decide application of DTO")
>> Signed-off-by: Michael Walle <mwalle@kernel.org>
>
> Thanks for explaining what's going on in such detail.
>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Please don't pick this up. There will be a v2 as this is breaking
any subsequent code in the loop.

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]