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  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, 15 Jan 2019 08:34:16 -0800
From:   Sean Christopherson <>
To:     Paolo Bonzini <>
Cc:     Qian Cai <>,,,,,,,,
Subject: Re: [PATCH] kvm: add proper frame pointer logic for vmx

On Tue, Jan 15, 2019 at 08:13:22AM +0100, Paolo Bonzini wrote:
> On 15/01/19 08:04, Qian Cai wrote:
> > 
> > 
> > On 1/15/19 1:44 AM, Qian Cai wrote:
> >> compilation warning since v5.0-rc1,
> >>
> >> arch/x86/kvm/vmx/vmx.o: warning: objtool: vmx_vcpu_run.part.17()+0x3171:
> >> call without frame pointer save/setup

The warning is complaining about vmx_vcpu_run() in vmx.c, not vmenter.S.
The rule being "broken" is that a call is made without creating a stack
frame, and vmx_vmenter() obviously makes no calls.

E.g., manually running objtool check:

    $ tools/objtool/objtool check arch/x86/kvm/vmx/vmenter.o
    $ tools/objtool/objtool check arch/x86/kvm/vmx/vmx.o
    arch/x86/kvm/vmx/vmx.o: warning: objtool: vmx_vcpu_run.part.19()+0x83e: call without frame pointer save/setup

I put "broken" in quotes because AFAICT we're not actually violating the
rule.  From tools/objtool/Documentation/stack-validation.txt:

   If it's a GCC-compiled .c file, the error may be because the function
   uses an inline asm() statement which has a "call" instruction.  An
   asm() statement with a call instruction must declare the use of the
   stack pointer in its output operand.  On x86_64, this means adding
   the ASM_CALL_CONSTRAINT as an output constraint:

     asm volatile("call func" : ASM_CALL_CONSTRAINT);

   Otherwise the stack frame may not get created before the call.

The asm() blob that calls vmx_vmenter() uses ASM_CALL_CONSTRAINT, and
the resulting asm output generates a frame pointer, e.g. this is from
the vmx.o that objtool warns on:

Dump of assembler code for function vmx_vcpu_run:
   0x0000000000007440 <+0>:     e8 00 00 00 00  callq  0x7445 <vmx_vcpu_run+5>
   0x0000000000007445 <+5>:     55      push   %rbp
   0x0000000000007446 <+6>:     48 89 e5        mov    %rsp,%rbp

The warning only shows up in certain configs, e.g. I was only able to
reproduce this using the .config provided by lkp.  Even explicitly
trigger the warning using my usual config.

And all that being said, I'm pretty sure this isn't related to the call
to vmx_vmenter() at all, but rather is something that was exposed by
removing __noclone from vmx_vcpu_run().

E.g. I still get the warning if I comment out the call to vmx_vmenter,
it just shifts to something else (and continues to shift I comment out
more calls).  The warning goes away if I re-add __noclone, regardless
of whether or not commit 2bcbd406715d ("Revert "compiler-gcc: disable
-ftracer for __noclone functions"") is applied.

0x6659 is in vmx_vcpu_run (./arch/x86/include/asm/spec-ctrl.h:43).
38       * Avoids writing to the MSR if the content/bits are the same
39       */
40      static inline
41      void x86_spec_ctrl_restore_host(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl)
42      {
43              x86_virt_spec_ctrl(guest_spec_ctrl, guest_virt_spec_ctrl, false);
44      }
46      /* AMD specific Speculative Store Bypass MSR data */
47      extern u64 x86_amd_ls_cfg_base;

What's really baffling is that we explicitly tell objtool to not do stack
metadata validation on vmx_vcpu_run():


As an aside, we might be able to remove STACK_FRAME_NON_STANDARD, assuming
we get this warning sorted out.

> >> Fixes: 453eafbe65f (KVM: VMX: Move VM-Enter + VM-Exit handling to
> >> non-inline sub-routines)
> > 
> > Oops, wrong fix. Back to square one.
> > 
> Hmm, maybe like this:
> diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S
> index bcef2c7e9bc4..33122fa9d4bd 100644
> --- a/arch/x86/kvm/vmx/vmenter.S
> +++ b/arch/x86/kvm/vmx/vmenter.S
> @@ -26,19 +26,17 @@ ENTRY(vmx_vmenter)
>  	ret
>  2:	vmlaunch
> +3:
>  	ret
> -3:	cmpb $0, kvm_rebooting
> -	jne 4f
> -	call kvm_spurious_fault
> -4:	ret
> -
>  	.pushsection .fixup, "ax"
> -5:	jmp 3b
> +4:	cmpb $0, kvm_rebooting
> +	jne 3b
> +	jmp kvm_spurious_fault
>  	.popsection
> -	_ASM_EXTABLE(1b, 5b)
> -	_ASM_EXTABLE(2b, 5b)
> +	_ASM_EXTABLE(1b, 4b)
> +	_ASM_EXTABLE(2b, 4b)
>  ENDPROC(vmx_vmenter)
> Paolo

Powered by blists - more mailing lists