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

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: <CAAhV-H6Cntwxo2XcPtB+zH0VE5J3N=Wb2Ad8RZ6DjwopGsXALw@mail.gmail.com>
Date: Fri, 5 Sep 2025 12:33:58 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: Tiezhu Yang <yangtiezhu@...ngson.cn>, Peter Zijlstra <peterz@...radead.org>, 
	Nathan Chancellor <nathan@...nel.org>, loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()

Hi, Josh,

On Fri, Sep 5, 2025 at 1:26 AM Josh Poimboeuf <jpoimboe@...nel.org> wrote:
>
> On Thu, Sep 04, 2025 at 10:17:11AM +0800, Huacai Chen wrote:
> > Hi, Josh,
> >
> > On Thu, Sep 4, 2025 at 3:17 AM Josh Poimboeuf <jpoimboe@...nel.org> wrote:
> > >
> > > On Mon, Sep 01, 2025 at 04:31:36PM +0800, Tiezhu Yang wrote:
> > > > On 2025/9/1 下午4:16, Peter Zijlstra wrote:
> > > > > On Mon, Sep 01, 2025 at 03:21:54PM +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 it is not a proper way.
> > > > > >
> > > > > > After more analysis, one simple way is to ignore these EFISTUB functions
> > > > > > in validate_branch() of objtool since they are useless for stack unwinder.
> > > > > >
> > > > >
> > > > > This is drivers/firmware/efi/libstub/loongarch.c:efi_boot_kernel(),
> > > > > right?
> > > > >
> > > > > Why not simply do something like:
> > > > >
> > > > > diff --git a/drivers/firmware/efi/libstub/loongarch.c b/drivers/firmware/efi/libstub/loongarch.c
> > > > > index 3782d0a187d1..082611a5f1f0 100644
> > > > > --- a/drivers/firmware/efi/libstub/loongarch.c
> > > > > +++ b/drivers/firmware/efi/libstub/loongarch.c
> > > > > @@ -81,4 +81,5 @@ 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);
> > > > > +   BUG();
> > > > >   }
> > > >
> > > > At the beginning, I did the above change, but no effect.
> > > >
> > > > The first thing is to remove the attribute __noreturn for
> > > > real_kernel_entry(), otherwise the compiler can not generate
> > > > instructions after that.
> > > >
> > > > But there is an argument in the previous RFC [1]:
> > > >
> > > > "From my point of view this is incorrect, this function is indeed a
> > > > noreturn function, and this modification makes LoongArch different to
> > > > other architectures."
> > > >
> > > > Josh suggested to do something so that the EFI stub code isn't linked into
> > > > vmlinux.o [2], it needs to modify the link process and seems too
> > > > complicated and expensive for this warning to some extent.
> > > >
> > > > So I did this change for objtool.
> > >
> > > I don't like adding these workarounds to objtool.  Is it really that
> > > complicated to link efistub separately?  That seems like the proper
> > > design.  vmlinux.o should only have real kernel code.
> > I don't think this is just a "workaround", ARM64, RISC-V and LoongArch
> > share the same logic in efistub which may be different from X86. When
> > ARM64 and RISC-V add objtool support, they will also need to ignore
> > the __efistub_ functions.
> >
> > The other patch is similar.
>
> Objtool expects/enforces certain rules.  One of them is that vmlinux.o
> is proper runtime kernel code.  efistub is not that.
>
> Is there some technical reason why vmlinux.o needs efistub linked in?
I think so. For example, EFISTUB prefer to directly use screen_info
that defined in vmlinux, see the comments in
drivers/firmware/efi/libstub/screen_info.c:

/*
 * There are two ways of populating the core kernel's struct
screen_info via the stub:
 * - using a configuration table, like below, which relies on the EFI init code
 *   to locate the table and copy the contents;
 * - by linking directly to the core kernel's copy of the global symbol.
 *
 * 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. The zboot decompressor can only use the configuration table
 * approach.
 */

Huacai

>
> --
> Josh