Re: [BUG report] spl: image size check fails in spl_load()

[Sean Anderson <seanga2@gmail.com>]

On 2/14/25 06:12, Anshul Dalal wrote:
> Hi all!
> 
> I was trying to implement falcon boot on TI AM62x EVM with the kernel image on
> SD card's filesystem but the following check in `_spl_load` at
> `include/spl_load.h:95` fails to -EIO as per the latest commit [89d3333]:
> 
> 	return read < spl_image->size ? -EIO : 0;
> 
> The check seems to be comparing the image size gathered from the header
> (spl_image->size) with the number of bytes read form the loader.
> 
>  From spl_load.h:
> 
> 	ret = spl_parse_image_header(spl_image, bootdev, header);
> 	if (ret)
> 		return ret;
> 
> 	base_offset = spl_image->offset;
> 	/* Only NOR sets this flag. */
> 	if (IS_ENABLED(CONFIG_SPL_NOR_SUPPORT) &&
> 	    spl_image->flags & SPL_COPY_PAYLOAD_ONLY)
> 		base_offset += sizeof(*header);
> 	image_offset = ALIGN_DOWN(base_offset, spl_get_bl_len(info));
> 	overhead = base_offset - image_offset;
> 	size = ALIGN(spl_image->size + overhead, spl_get_bl_len(info));
> 
> 	read = info->read(info, offset + image_offset, size,
> 			  map_sysmem(spl_image->load_addr - overhead, size));
> 
> 	if (read < 0)
> 		return read;
> 
> 	return read < spl_image->size ? -EIO : 0;
> 
> During kernel build process the header size is computed including the BSS
> whereas it's removed when creating the uncompressed image. Therefore the size
> of the uncompressed image on filesystem will be smaller than the size specified
> in the header. Which leads to failure of the above check.
> 
>  From linux kernel's `arch/arm64/kernel/image.h:63`:
> 
> 	#define HEAD_SYMBOLS						\
> 		DEFINE_IMAGE_LE64(_kernel_size_le, _end - _text);	\
> 		DEFINE_IMAGE_LE64(_kernel_flags_le, __HEAD_FLAGS);
> 
> Disabling the check leads to a successful boot directly to the kernel.
> Therefore it seems like the check is non functional as the size in the kernel
> header does not correspond with the file size of the kernel image.

Did this work before v2024.04?

How exactly are you loading your image? E.g. what are the values of

CONFIG_SPL_OS_BOOT
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
CONFIG_SPL_FALCON_BOOT_MMCSD
CONFIG_SPL_FS_FAT
CONFIG_SPL_FS_EXT4
CONFIG_SPL_FS_LOAD_PAYLOAD_NAME
CONFIG_SUPPORT_EMMC_BOOT

 From what I can tell, the OS_BOOT path should not call spl_load in the first place.

In any case, the root problem is that the size reported by the kernel is actually the
space the kernel will need when it is loaded, and not the size of the data to load
(which we need). So if we have a short read, we have no way of knowing if the filesystem
is corrupt, the image was truncated while writing, or if it's just missing the bss. And
we still have to rely of the image size so that we can load from e.g. NAND or SPI where
there is no filesystem.

One way to fix this could be to move the length check to spl_load_info->read. This would
involve updating all the callers and callees.

--Sean