| 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
| ||
|
Message-ID: <9508dbf5-6432-f459-f273-8b327d51a055@loongson.cn>
Date: Sat, 11 Oct 2025 16:13:31 +0800
From: Tiezhu Yang <yangtiezhu@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: Ard Biesheuvel <ardb@...nel.org>, 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/10/11 下午3:42, Huacai Chen wrote:
> On Sat, Oct 11, 2025 at 3:29 PM Tiezhu Yang <yangtiezhu@...ngson.cn> wrote:
>>
>> On 2025/10/11 上午11:40, Ard Biesheuvel wrote:
>>> On Fri, 10 Oct 2025 at 19:54, Huacai Chen <chenhuacai@...nel.org> wrote:
>>>>
>>>> On Sat, Oct 11, 2025 at 9:13 AM Tiezhu Yang <yangtiezhu@...ngson.cn> wrote:
>>>>>
>>>>> On 2025/10/11 上午12:25, Ard Biesheuvel wrote:
>>>>> ...
>>>>>> Why do we need both (1) and (2)?
>>>>>
>>>>> Not both, either (1) or (2).
>>>>> Which one do you prefer? Or any other suggestions?
>>>>>
>>>>> Taking all of the considerations in balance, we should decide
>>>>> what is the proper way.
>>>> As a summary, there are three methods:
>>>> (1) Only link libstub with vmlinux.o during the final vmlinux link.
>>>> (2) Remove the attribute __noreturn for real_kernel_entry() and add while (1).
>>>> (3) Ignore "__efistub_" prefix in objtool.
>>>>
>>>> Josh prefers method (1), I prefer method (3) but also accept method
>>>> (1) if it is not only specific to loongarch.
>>>>
>>>
>>> This is a false positive warning in objtool, which complains about a
>>> function that falls through, even though that can never happen in
>>> reality.
>>>
>>> To me, it is not acceptable to modify how vmlinux.o is constructed
>>> also for other architectures, in order to hide some of its constituent
>>> parts from objtool, which do not use objtool to begin with.
>>>
>>>
>>> If you are not willing to fix objtool, I suggest fixing the loongarch
>>> code like this:
>>
>> Thank you.
>>
>>> --- 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,6 @@
>>>
>>> real_kernel_entry(true, (unsigned long)cmdline_ptr,
>>> (unsigned long)efi_system_table);
>>> +
>>> + return EFI_LOAD_ERROR;
>>> }
>>
>> I tested the above changes, the falls through objtool warning can
>> be fixed because efi_boot_kernel() ends with a return instruction,
>> I think this is reasonable.
>>
>> efi_boot_kernel() has a return value, there are "return status" in
>> other parts of efi_boot_kernel(), it should also return at the end
>> of efi_boot_kernel() in theory, although we should never get here.
>>
>> If there are more comments, please let me know.
> I still don't want LoongArch to be a special case, which means
> efi_boot_kernel() in fdt.c, jump_kernel_func in riscv.c and
> enter_kernel in arm64.c should also be modified.
Here is the diff:
----- 8< -----
diff --git a/drivers/firmware/efi/libstub/arm64.c
b/drivers/firmware/efi/libstub/arm64.c
index e57cd3de0a00..f3a1bb86bf8d 100644
--- a/drivers/firmware/efi/libstub/arm64.c
+++ b/drivers/firmware/efi/libstub/arm64.c
@@ -128,11 +128,11 @@ unsigned long __weak primary_entry_offset(void)
return 0;
}
-void __noreturn efi_enter_kernel(unsigned long entrypoint,
- unsigned long fdt_addr,
- unsigned long fdt_size)
+void efi_enter_kernel(unsigned long entrypoint,
+ unsigned long fdt_addr,
+ unsigned long fdt_size)
{
- void (* __noreturn enter_kernel)(u64, u64, u64, u64);
+ void (*enter_kernel)(u64, u64, u64, u64);
enter_kernel = (void *)entrypoint + primary_entry_offset();
enter_kernel(fdt_addr, 0, 0, 0);
diff --git a/drivers/firmware/efi/libstub/efistub.h
b/drivers/firmware/efi/libstub/efistub.h
index f5ba032863a9..b4c716df4d47 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -1129,9 +1129,9 @@ efi_status_t efi_stub_common(efi_handle_t handle,
efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char
**cmdline_ptr);
-asmlinkage void __noreturn efi_enter_kernel(unsigned long entrypoint,
- unsigned long fdt_addr,
- unsigned long fdt_size);
+asmlinkage void efi_enter_kernel(unsigned long entrypoint,
+ unsigned long fdt_addr,
+ unsigned long fdt_size);
void efi_handle_post_ebs_state(void);
diff --git a/drivers/firmware/efi/libstub/fdt.c
b/drivers/firmware/efi/libstub/fdt.c
index 6a337f1f8787..7ece39ed70f6 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -358,7 +358,9 @@ efi_status_t efi_boot_kernel(void *handle,
efi_loaded_image_t *image,
efi_handle_post_ebs_state();
efi_enter_kernel(kernel_addr, fdt_addr, fdt_totalsize((void
*)fdt_addr));
+
/* not reached */
+ return EFI_LOAD_ERROR;
}
void *get_fdt(unsigned long *fdt_size)
diff --git a/drivers/firmware/efi/libstub/loongarch.c
b/drivers/firmware/efi/libstub/loongarch.c
index 3782d0a187d1..88c500aeed86 100644
--- a/drivers/firmware/efi/libstub/loongarch.c
+++ b/drivers/firmware/efi/libstub/loongarch.c
@@ -10,8 +10,8 @@
#include "efistub.h"
#include "loongarch-stub.h"
-typedef void __noreturn (*kernel_entry_t)(bool efi, unsigned long cmdline,
- unsigned long systab);
+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);
+
+ /* not reached */
+ return EFI_LOAD_ERROR;
}
diff --git a/drivers/firmware/efi/libstub/riscv.c
b/drivers/firmware/efi/libstub/riscv.c
index f66f33ceb99e..6589e8f5cf1a 100644
--- a/drivers/firmware/efi/libstub/riscv.c
+++ b/drivers/firmware/efi/libstub/riscv.c
@@ -11,7 +11,7 @@
#include "efistub.h"
-typedef void __noreturn (*jump_kernel_func)(unsigned long, unsigned long);
+typedef void (*jump_kernel_func)(unsigned long, unsigned long);
static unsigned long hartid;
@@ -81,7 +81,7 @@ unsigned long __weak stext_offset(void)
return 0;
}
-void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned
long fdt,
+void efi_enter_kernel(unsigned long entrypoint, unsigned long fdt,
unsigned long fdt_size)
{
unsigned long kernel_entry = entrypoint + stext_offset();
Do the above changes in a single patch?
Or in two patches?
One is to modify drivers/firmware/efi/libstub/loongarch.c,
the other one is to modify the arm64 and riscv.
Let us wait for more review comments.
Thanks,
Tiezhu
Powered by blists - more mailing lists