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: <qvatvh7rixtdtaflqtgphlva7kkr47drijklkvmae3xh54vn6y@y5v75lwgjdyu>
Date: Wed, 3 Sep 2025 12:17:29 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Tiezhu Yang <yangtiezhu@...ngson.cn>
Cc: Peter Zijlstra <peterz@...radead.org>, 
	Huacai Chen <chenhuacai@...nel.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()

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.

-- 
Josh

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ