Hi Marek, On Wed, 19 Nov 2025 at 17:20, Marek Vasut <[email protected]> wrote: > > On 11/18/25 4:47 AM, Simon Glass wrote: > > Hello Simon, > > >>>> 2294 } else if (load_op != FIT_LOAD_IGNORED && image_type == > >>>> IH_TYPE_FLATDT && > >>>> 2295 ((uintptr_t)buf & 7)) { > >>>> 2296 loadbuf = memalign(8, len); > >>>> 2297 load = map_to_sysmem(loadbuf); > >>>> 2298 memcpy(loadbuf, buf, len); > >>>> > >>>> ... > >>>> > >>>> 2309 /* verify that image data is a proper FDT blob */ > >>>> 2310 if (load_op != FIT_LOAD_IGNORED && image_type == > >>>> IH_TYPE_FLATDT && > >>>> 2311 fdt_check_header(loadbuf)) { <----------------- this > >>>> 2312 puts("Subimage data is not a FDT\n"); > >>>> 2313 return -ENOEXEC; > >>>> 2314 } > >>>> > >>>>> Perhaps we should deprecate FITs with internal data, too? > >>>> We cannot break compatibility and stop supporting old fitImage, so this > >>>> is irrelevant here. > >>> > >>> OK, so please create a function which can detect an FDT header without > >>> it being aligned, like the other code you wrote. Then it will be safe > >>> to call that here, even if unaligned. > >> But we actually do want to detect unaligned broken FDT header and either > >> fix it up or stop processing, we don't want to perpetuate handling of > >> broken FDTs and pretend that is OK, it shouldn't be I think. Hence this > >> fixup. > > > > The decision as to whether something is an FDT is made a lot earlier > > than the actual processing of it. For the former there is no need to > > allocate and copy. For the latter we need to. > > The test whether the FDT is valid cannot be done earlier, this place is > literally the first place when the FDT is decompressed/loaded to the > target location.
Yes, this is the place where you are testing whether it is valid. Later, the FDT is actually used. It just isn't right to allocate memory in fit_image_load() - it is for loading an image. > > > Anyway, you are doing the patches, so do what you think is best. But > > please create a function for this, rather than lots of little > > hand-crafted checks around the place. > I am confused, function for what exactly ? I believe the code you should patch is in boot_get_fdt_fit(), after fit_image_load() returns. That is where the processing happens. That is where the function I suggested (and tried to write) could be called, to get an FDT that can be used. It also happens to be one of the places where boot_relocate_fdt() is called, which could mean you don't need to allocate memory. Regards, Simon
