Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk

[Tom Rini <trini@konsulko.com>]

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

On Sat, Jan 22, 2022 at 05:31:18PM +0100, Pali Rohár wrote:
> On Friday 21 January 2022 21:15:43 Tom Rini wrote:
> > On Sat, Jan 22, 2022 at 02:44:22AM +0100, Pali Rohár wrote:
> > > On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> > > > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> > > > 
> > > > > If image backend provides verify_header callback then call it after writing
> > > > > image to disk. This ensures that written image is correct.
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > Reviewed-by: Stefan Roese <sr@denx.de>
> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 41 insertions(+)
> > > > 
> > > > This breaks a number of platforms such as ls1021atwr_sdcard_qspi and
> > > > it's not clear to me why exactly.
> > > 
> > > Maybe they were already broken and this patch just detected it?
> > > Or verify_header callback for particular image type is reject valid
> > > image?
> > > 
> > > Do you have some pointers to failed build logs?
> > 
> > Try building for ls1021atwr_sdcard_qspi with your patch applied, the
> > only new thing that's shown in the logs is the error message.
> 
> So... I have tried following without this patch:
> 
> $ make ls1021atwr_sdcard_qspi_defconfig
> $ make CROSS_COMPILE=arm-linux-gnueabi- -j8
> 
> It generated file spl/u-boot-spl.pbl without error. Now I called -l on
> this generated file for type pblimage and I got following output:
> 
> $ ./tools/dumpimage -T pblimage -l spl/u-boot-spl.pbl
> GP Header: Size aa55aa55 LoadAddr 1ee0100
> 
> $ ./tools/mkimage -T pblimage -l spl/u-boot-spl.pbl
> GP Header: Size aa55aa55 LoadAddr 1ee0100
> 
> "GP Header:" line is from the TI OMAP image backend type gpimage or type
> omapimage (implemented in file gpimage-common.c).
> 
> So it means that files generated by ls1021atwr_sdcard_qspi are already
> broken and my patch just detected it. Or it is also possible that
> validation code in pblimage.c file is incorrect and broken.
> 
> What to do with it now?

Thanks for digging.  This is a problem for a number of the ls1021,
ls1043 and ls1046 platforms, so lets add some maintainers there.

-- 
Tom

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