linux-kernel - Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a _

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: <9eb77be3a64c25b3264d5011a93242d006b88627.camel@intel.com>
Date:   Thu, 14 Sep 2023 07:54:10 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "peterz@...radead.org" <peterz@...radead.org>
CC:     "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        lkp <lkp@...el.com>,
        "oe-kbuild-all@...ts.linux.dev" <oe-kbuild-all@...ts.linux.dev>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool:
 __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn
 annotation

On Thu, 2023-09-14 at 09:29 +0200, Peter Zijlstra wrote:
> On Thu, Sep 14, 2023 at 03:21:29AM +0000, Huang, Kai wrote:
> > On Thu, 2023-09-14 at 01:23 +0000, Huang, Kai wrote:
> > > On Thu, 2023-09-14 at 09:05 +0800, kernel test robot wrote:
> > > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/tdx
> > > > head:   7b804135d4d1f0a2b9dda69c6303d3f2dcbe9d37
> > > > commit: c641cfb5c157b6c3062a824fd8ba190bf06fb952 [8/12] x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL
> > > > config: x86_64-rhel-8.3-bpf (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/config)
> > > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/reproduce)
> > > > 
> > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > > the same patch/commit), kindly add following tags
> > > > > Reported-by: kernel test robot <lkp@...el.com>
> > > > > Closes: https://lore.kernel.org/oe-kbuild-all/202309140828.9RdmlH2Z-lkp@intel.com/
> > > > 
> > > > All warnings (new ones prefixed by >>):
> > > > 
> > > > > > vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
> > > > 
> > > 
> > > Hmm.. The __tdx_hypercall_failed() is already annotated with __noreturn (I
> > > explicitly added it to silent such warning):
> > > 
> > > /* Called from __tdx_hypercall() for unrecoverable failure */
> > > noinstr void __noreturn __tdx_hypercall_failed(void)
> > > {
> > >         instrumentation_begin();
> > >         panic("TDVMCALL failed. TDX module bug?");
> > > }
> > > 
> > > Not sure why the objtool is still complaining this?
> > > 
> > 
> > It appears the __noreturn must be annotated to the function declaration but not
> > the function body.  I'll send out the fix as soon as I confirm the fix with LKP.
> 
> FWIW, the reason being that...
> 
> The point of noreturn is that the caller should know to stop generating
> code. For that the declaration needs the attribute, because call sites
> typically do not have access to the function definition in C.

Ah that makes perfect sense.  Thanks!

Then I assume we don't need to annotate __noreturn in the function body, but
only in the declaration?  Because the compiler must already have seen the
declaration when it generates the code for the function body.

Btw, I happened to notice that the objtool documentation suggests that we should
also add the the function to tools/objtool/noreturns.h:

3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation

   The call from foo() to bar() doesn't return, but bar() is missing the
   __noreturn annotation.  NOTE: In addition to annotating the function
   with __noreturn, please also add it to tools/objtool/noreturns.h.

Is it a behaviour that we still need to follow?

I am asking because the old kernel code doesn't have it so perhaps I am missing
something here.