Re: [PATCH v2 07/10] efi/libstub: riscv: Use struct efi_image_info to access image properties

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Critical] efi/libstub: riscv: wrong image base and entry offset from
  _text
--

commit 66c3f5bb26141f8dc9e6a6b6eea665f0e9187c52
Author: Ard Biesheuvel <ardb@kernel.org>

efi/libstub: riscv: Use struct efi_image_info to access image properties

This commit converts the RISC-V EFI stub to use struct efi_image_info
helpers instead of linker script symbol aliases to determine the kernel
image base, sizes, and entry offset.

> diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
> index 5cbc50a814e11..e8d66f6fe8e03 100644
> --- a/drivers/firmware/efi/libstub/riscv-stub.c
> +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> @@ -20,7 +19,7 @@ unsigned long stext_offset(void)
>  	 * of the file representation. So instead, we need to jump to the
>  	 * actual entrypoint in the .text region of the image.
>  	 */
> -	return _start_kernel - _start;
> +	return efistub_kernel_entry_offset(efistub_get_image_info());
>  }
>  
>  efi_status_t handle_kernel_image(unsigned long *image_addr,
> @@ -30,13 +29,14 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>  				 efi_loaded_image_t *image,
>  				 efi_handle_t image_handle)
>  {
> -	unsigned long kernel_size, kernel_codesize, kernel_memsize;
> +	auto image_info			= efistub_get_image_info();
> +	unsigned long kernel_base	= efistub_kernel_base(image_info);
> +	unsigned long kernel_size	= efistub_kernel_size(image_info);
> +	unsigned long kernel_codesize	= efistub_kernel_codesize(image_info);
> +	unsigned long kernel_memsize	= efistub_kernel_memsize(image_info);
>  	efi_status_t status;
>  
> -	kernel_size = _edata - _start;
> -	kernel_codesize = __inittext_end - _start;
> -	kernel_memsize = kernel_size + (_end - _edata);
> -	*image_addr = (unsigned long)_start;
> +	*image_addr = kernel_base;
>  	*reserve_size = kernel_memsize;

[Severity: Critical]
Does using struct efi_image_info here base the RISC-V image calculations on
_text instead of _start?

struct efi_image_info records _text_offset using REL_REF(_text), and the
efistub_kernel_*() helpers compute the base address, image sizes, and
efi_primary_entry (_start_kernel) offset relative to _text.  On RISC-V,
however, _start and the .head.text section (which contains _start_kernel)
are placed before _text:

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

	.text : {
		_text = .;
		_stext = .;

As a result, handle_kernel_image() sets *image_addr to _text and omits
.head.text when efi_kaslr_relocate_kernel() checks alignment and copies the
image, while stext_offset() returns _start_kernel - _text (a negative
s32 offset that sign-extends to unsigned long), causing efi_enter_kernel()
to jump before the start of the image buffer and crash at boot.

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