Re: [PATCH v2 09/10] efi/zboot: Disentangle efi_remap_image() from efi_cache_sync_image()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] efi/zboot: broken image_info offset and code_size in
  efi_zboot_remap_image()
--

commit 51947c84bec76536d55cfa9120873713a58ae31b
Author: Ard Biesheuvel <ardb@kernel.org>

efi/zboot: Disentangle efi_remap_image() from efi_cache_sync_image()

This commit moves the efi_cache_sync_image() and efi_remap_image() calls out
of the architecture-specific and decompressor paths into a new
efi_zboot_remap_image() helper invoked from efi_zboot_entry().

> diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
> index c6071b0e8f1a4..25ff5960771ca 100644
> --- a/drivers/firmware/efi/libstub/zboot.c
> +++ b/drivers/firmware/efi/libstub/zboot.c
> @@ -38,6 +38,17 @@ const struct efi_image_info *zboot_get_image_info(unsigned long image_base)
>  	return (void *)image_base + efistub_image_info_offset;
>  }
>  
> +static efi_status_t efi_zboot_remap_image(const struct efi_image_info *image_info,
> +					  unsigned long image_base,
> +					  unsigned long image_size)
> +{
> +	unsigned long code_size = efistub_kernel_codesize(image_info);
> +
> +	efi_cache_sync_image(image_base, code_size);
> +
> +	return efi_remap_image(image_base, image_size, code_size);
> +}
> +
>  asmlinkage efi_status_t __efiapi
>  efi_zboot_entry(efi_handle_t handle, efi_system_table_t *systab)
>  {
> @@ -105,7 +116,8 @@ efi_zboot_entry(efi_handle_t handle, efi_system_table_t *systab)
>  		goto error;
>  
>  	auto image_info = zboot_get_image_info(image_base);
> -	status = efi_stub_common(handle, image, image_base, cmdline_ptr,
> +	status = efi_zboot_remap_image(image_info, image_base, alloc_size) ?:
> +	         efi_stub_common(handle, image, image_base, cmdline_ptr,
>  				 image_info);

[Severity: High]
Does calling efi_zboot_remap_image() here cause a boot regression on RISC-V
CONFIG_EFI_ZBOOT kernels when EFI_MEMORY_ATTRIBUTE_PROTOCOL is present?

Previously, efi_remap_image() in the zboot path was only called on arm64, and
zboot.c never dereferenced image_info.  Now efi_zboot_entry() calls
efi_zboot_remap_image() for all zboot architectures and aborts the boot if it
fails.

On RISC-V, the binary Image decompressed to image_base starts at _start, while
_text is placed PAGE_SIZE bytes later, and __inittext_end is defined before
section and PE/COFF alignment:

arch/riscv/kernel/vmlinux.lds.S:
	_start = .;
	HEAD_TEXT_SECTION
	. = ALIGN(PAGE_SIZE);

	.text : {
		_text = .;
    ...
	__inittext_end = .;
	. = ALIGN(SECTION_ALIGN);
#ifdef CONFIG_EFI
	. = ALIGN(PECOFF_SECTION_ALIGNMENT);
	__pecoff_text_end = .;
#endif

Because get_image_info_offset in drivers/firmware/efi/libstub/Makefile.zboot
computes efistub_image_info_offset relative to _text,
zboot_get_image_info(image_base) points PAGE_SIZE bytes before the actual
struct efi_image_info, so efistub_kernel_codesize(image_info) reads unrelated
image bytes as code_size.  Even with the offset fixed,
__inittext_end - _text omits the leading PAGE_SIZE bytes between _start and
_text and is not page-aligned, causing efi_remap_image() to fail with
EFI_INVALID_PARAMETER or remap the wrong range.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110121731.2728824-12-ardb+git@google.com?part=9