linux-kernel - Re: [PATCH v3 1/3] objtool/LoongArch: Fix fall through warning about efistub

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <64530e20-9bc7-0c0b-d6ea-a4c7df3e85d9@loongson.cn>
Date: Thu, 18 Sep 2025 09:44:24 +0800
From: Tiezhu Yang <yangtiezhu@...ngson.cn>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Huacai Chen
 <chenhuacai@...nel.org>, loongarch@...ts.linux.dev,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/3] objtool/LoongArch: Fix fall through warning about
 efistub

On 2025/9/18 上午6:32, Josh Poimboeuf wrote:
> On Wed, Sep 17, 2025 at 07:27:14PM +0800, Tiezhu Yang wrote:
>> When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
>> the following objtool warning:
>>
>>    vmlinux.o: warning: objtool: __efistub_efi_boot_kernel()
>>    falls through to next function __efistub_exit_boot_func()
>>
>> This is because efi_boot_kernel() doesn't end with a return instruction
>> or an unconditional jump, then objtool has determined that the function
>> can fall through into the next function.
>>
>> At the beginning, try to do something to make efi_boot_kernel() ends with
>> an unconditional jump instruction, but this modification seems not proper
>> and makes LoongArch different to other architectures.
>>
>> Furthermore, it is not proper to link efistub separately for LoongArch,
>> ARM64 and RISC-V too due to there is technical reason why vmlinux.o needs
>> efistub linked in.
>>
>> For example, there are two ways of populating the core kernel's struct
>> screen_info via the stub, the latter is preferred because it makes the
>> EFIFB earlycon available very early, but it only works if the EFI stub
>> is part of the core kernel image itself. For more information, please
>> see the comments in drivers/firmware/efi/libstub/screen_info.c.
> 
> Hm?  Of course libstub needs to be linked into *vmlinux*.  But that
> doesn't mean it needs to be in vmlinux.o.
> 
> Why not build libstub.a separately, and then link it with vmlinux.o
> during the final vmlinux link, as x86 does?

If this change is not OK, what do you think of the following changes?
Which one do you prefer?

(1) libstub doesn't link to vmlinux.o, only link libstub with vmlinux.o
during the final vmlinux link.

----->8-----
diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index a3a9759414f4..919c1970ce14 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -164,7 +164,6 @@ CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) 
$(KBUILD_CFLAGS) -dM -E -x c /dev
  endif

  libs-y += arch/loongarch/lib/
-libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a

  drivers-y              += arch/loongarch/crypto/

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 51367c2bfc21..c664bfb9b15f 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -69,6 +69,12 @@ vmlinux_link()
                 libs="${KBUILD_VMLINUX_LIBS}"
         fi

+       if [ "${SRCARCH}" = "loongarch" ]; then
+               if is_enabled CONFIG_EFI_STUB; then
+                       libs="${libs} drivers/firmware/efi/libstub/lib.a"
+               fi
+       fi
+
         if is_enabled CONFIG_GENERIC_BUILTIN_DTB; then
                 objs="${objs} .builtin-dtbs.o"
         fi

https://lore.kernel.org/loongarch/49ceb19c-6107-d026-3ae6-ae897d1fcae4@loongson.cn/

(2) make efi_boot_kernel() ends with an unconditional jump instruction.

----->8-----
diff --git a/drivers/firmware/efi/libstub/loongarch.c 
b/drivers/firmware/efi/libstub/loongarch.c
index 3782d0a187d1..e309fd78fca7 100644
--- a/drivers/firmware/efi/libstub/loongarch.c
+++ b/drivers/firmware/efi/libstub/loongarch.c
@@ -10,7 +10,7 @@
  #include "efistub.h"
  #include "loongarch-stub.h"

-typedef void __noreturn (*kernel_entry_t)(bool efi, unsigned long cmdline,
+typedef void (*kernel_entry_t)(bool efi, unsigned long cmdline,
  					  unsigned long systab);

  efi_status_t check_platform_features(void)
@@ -81,4 +81,7 @@ efi_status_t efi_boot_kernel(void *handle, 
efi_loaded_image_t *image,

  	real_kernel_entry(true, (unsigned long)cmdline_ptr,
  			  (unsigned long)efi_system_table);
+
+	/* We should never get here */
+	while (1);
  }

https://lore.kernel.org/loongarch/20250826064631.9617-2-yangtiezhu@loongson.cn/

Thanks,
Tiezhu