linux-kernel - Re: objtool warnings for kernel/trace/trace_selftest

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]

Date:   Tue, 18 Dec 2018 19:44:01 -0800
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Andi Kleen <ak@...ux.intel.com>
Cc:     Josh Poimboeuf <jpoimboe@...hat.com>,
        Martin Jambor <mjambor@...e.cz>,
        Miroslav Benes <mbenes@...e.cz>,
        Steven Rostedt <rostedt@...dmis.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Arnd Bergmann <arnd@...db.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Nadav Amit <namit@...are.com>
Subject: Re: objtool warnings for kernel/trace/trace_selftest_dynamic.o

+cc Nadav and Paolo

On Tue, Dec 18, 2018 at 01:20:42PM -0800, Andi Kleen wrote:
> > I whittled it down to a small test case.  It turns out the problem is
> > caused by the "__optimize__("no-tracer")" atribute, which is used by our
> > __noclone macro:
> > 
> > 
> > # if __has_attribute(__optimize__)
> > #  define __noclone                     __attribute__((__noclone__, __optimize__("no-tracer")))
> > # else
> > #  define __noclone                     __attribute__((__noclone__))
> > # endif
> 
> Ah interesting. That makes sense. Thanks for tracking that down.
> 
> > 
> > commit 95272c29378ee7dc15f43fa2758cb28a5913a06d
> > Author: Paolo Bonzini <pbonzini@...hat.com>
> > Date:   Thu Mar 31 09:38:51 2016 +0200
> > 
> >     compiler-gcc: disable -ftracer for __noclone functions
> > 
> >     -ftracer can duplicate asm blocks causing compilation to fail in
> >     noclone functions.  For example, KVM declares a global variable
> >     in an asm like
> > 
> >         asm("2: ... \n
> >              .pushsection data \n
> >              .global vmx_return \n
> >              vmx_return: .long 2b");
> > 
> >     and -ftracer causes a double declaration.
> 
> Ok. So the real solution would be to add a no tracer to the KVM
> function only.
> 
> But of course it will drop its frame pointer. So if you rely on
> frame pointer for live patching you would still have a problem.
> 
> Just fixing the ftrace case independently may not be useful?
> 
> ftrace here really not interesting at all for live patching because
> the ftracing self test only happens at boot, when there is no live patching.
> Likely it's even in __init code.
> 
> KVM on the other hand is quite important and actually happens at boot.
> 
> So I would concentrate on fixing that one instead, but leave ftrace
> alone.
> 
> My suggestion to fix KVM would be to drop the inline assembler
> and use out of line assembler in a separate file. Then disabling
> the tracer wouldn't be needed.
> 
> VM entries are very expensive anyways, I doubt avoiding a single call/ret
> makes any difference at all.

The KVM code in question isn't inline asm for performance, but rather
so that it can reference struct offsets when marshalling data between
the CPU and software structs when loading/saving guest state.  The
structs in question are local to VMX, i.e. don't belong in asm-offsets.h,
and until now there hasn't been a need to move away from inline asm.

Nadav recently pointed out that the __noclone attribute results in
vmx_vcpu_run() failing to inline even trivial code.  So now there's
two reasons to get rid of __noclone on vmx_vcpu_run().

But rather than move the entire asm blob wholesale into a separate asm
file, we can move just VMLAUNCH+VMRESUME along with a minimal amount
of wrapper code.  I have working code, just need to write a changelog.
I'll send a series tomorrow for the VMX change and a revert of commit
95272c29378e ("compiler-gcc: disable -ftracer for __noclone functions").

[1] https://patchwork.kernel.org/patch/8707981/#21817015