linux-kernel - Re: [PATCH v2] efistub: Only link libstub to final vmlinux

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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fec0c03d-9d8c-89a3-886a-1adc22e59b66@loongson.cn>
Date: Thu, 9 Oct 2025 15:27:39 +0800
From: Tiezhu Yang <yangtiezhu@...ngson.cn>
To: Ard Biesheuvel <ardb@...nel.org>, Huacai Chen <chenhuacai@...nel.org>
Cc: Josh Poimboeuf <jpoimboe@...nel.org>, loongarch@...ts.linux.dev,
 linux-arm-kernel@...ts.infradead.org, linux-riscv@...ts.infradead.org,
 linux-efi@...r.kernel.org, linux-kbuild@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] efistub: Only link libstub to final vmlinux

On 2025/9/28 下午10:41, Ard Biesheuvel wrote:
> On Sun, 28 Sept 2025 at 16:39, Ard Biesheuvel <ardb@...nel.org> wrote:
>>
>> On Sun, 28 Sept 2025 at 15:52, Huacai Chen <chenhuacai@...nel.org> wrote:
>>>
>>> Hi, Ard,
>>>
>>> On Sun, Sep 28, 2025 at 9:42 PM Ard Biesheuvel <ardb@...nel.org> wrote:
>>>>
>>>> On Sun, 28 Sept 2025 at 10:55, Tiezhu Yang <yangtiezhu@...ngson.cn> wrote:
>>>>>
>>>>> When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
>>>>> the following objtool warning on LoongArch:
>>>>>
>>>>>    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.
>>>>>
>>>>> Since the efistub functions are useless for stack unwinder, they can be
>>>>> ignored by objtool. After many discussions, no need to link libstub to
>>>>> the vmlinux.o, only link libstub to the final vmlinux.
>>>>>
>>>>
>>>> Please try keeping these changes confined to arch/loongarch. This
>>>> problem does not exist on other architectures, and changing the way
>>>> vmlinux is constructed might create other issues down the road.

This was discussed and tested in the v3 patch, it only touches the code
of LoongArch and works well like this:

diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index dc5bd3f1b8d2..f34b416f5ca2 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -169,7 +169,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 433849ff7529..ed94871c3606 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/pq4h7jgndnt6p45lj4kgubxjd5gidfetugcuf5rcxzxxanzetd@6rrlpjnjsmuy/

>>> ARM, RISC-V and LoongArch do things exactly in the same way. Now
>>> LoongArch is the first of the three to enable objtool, so we meet the
>>> problem first.
>>>
>>> But yes, I also don't want to change the way of constructing vmlinux.
>>> So I prefer the earliest way to fix this problem.
>>> https://lore.kernel.org/loongarch/CAAhV-H7fRHGFVKV8HitRgmuoDPt5ODt--iSuV0EmeeUb9d5FNw@mail.gmail.com/T/#meef7411abd14f4c28c85e686614aa9211fccdca0
>>>
>>
>> Can we just drop the __noreturn annotation from kernel_entry_t, and
>> return EFI_SUCCESS from efi_boot_kernel()?

This was done in the early RFC patch, it works well like this:

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/

> ... or add efi_boot_kernel() to ./tools/objtool/noreturns.h?

It can not fix the objtool warning.

Are you OK with the following changes?

(1) libstub doesn't link to vmlinux.o, only link libstub with vmlinux.o 
during the final vmlinux link, keep the changes confined to LoongArch, 
no need to be something more generic.

diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index dc5bd3f1b8d2..f34b416f5ca2 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -169,7 +169,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 433849ff7529..ed94871c3606 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

(2) remove the attribute __noreturn for real_kernel_entry() and add
"while (1);" at the end of efi_boot_kernel().

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);
  }

Taking all of the considerations in balance, what should to do?

Thanks,
Tiezhu