All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Alexander Potapenko <glider@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	syzbot <syzbot+3f29ca2efb056a761e38@syzkaller.appspotmail.com>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	KVM list <kvm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	wanpengli@tencent.com, the arch/x86 maintainers <x86@kernel.org>
Subject: Re: BUG: unable to handle kernel NULL pointer dereference in handle_external_interrupt_irqoff
Date: Mon, 23 Mar 2020 09:39:25 -0700	[thread overview]
Message-ID: <20200323163925.GP28711@linux.intel.com> (raw)
In-Reply-To: <CAG_fn=WYtSoyi63ACaz-ya=Dbi+BFU-_mADDpL6gQvDimQscmw@mail.gmail.com>

On Mon, Mar 23, 2020 at 05:31:15PM +0100, Alexander Potapenko wrote:
> On Mon, Mar 23, 2020 at 9:18 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On 22/03/20 07:59, Dmitry Vyukov wrote:
> > >
> > > The commit range is presumably
> > > fb279f4e238617417b132a550f24c1e86d922558..63849c8f410717eb2e6662f3953ff674727303e7
> > > But I don't see anything that says "it's me". The only commit that
> > > does non-trivial changes to x86/vmx seems to be "KVM: VMX: check
> > > descriptor table exits on instruction emulation":
> >
> > That seems unlikely, it's a completely different file and it would only
> > affect the outside (non-nested) environment rather than your own kernel.
> >
> > The only instance of "0x86" in the registers is in the flags:
> >
> > > RSP: 0018:ffffc90001ac7998 EFLAGS: 00010086
> > > RAX: ffffc90001ac79c8 RBX: fffffe0000000000 RCX: 0000000000040000
> > > RDX: ffffc9000e20f000 RSI: 000000000000b452 RDI: 000000000000b453
> > > RBP: 0000000000000ec0 R08: ffffffff83987523 R09: ffffffff811c7eca
> > > R10: ffff8880a4e94200 R11: 0000000000000002 R12: dffffc0000000000
> > > R13: fffffe0000000ec8 R14: ffffffff880016f0 R15: fffffe0000000ecb
> > > FS:  00007fb50e370700(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 000000000000005c CR3: 0000000092fc7000 CR4: 00000000001426f0
> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> >
> > That would suggest a miscompilation of the inline assembly, which does
> > push the flags:
> >
> > #ifdef CONFIG_X86_64
> >                 "mov %%" _ASM_SP ", %[sp]\n\t"
> >                 "and $0xfffffffffffffff0, %%" _ASM_SP "\n\t"
> >                 "push $%c[ss]\n\t"
> >                 "push %[sp]\n\t"
> > #endif
> >                 "pushf\n\t"
> >                 __ASM_SIZE(push) " $%c[cs]\n\t"
> >                 CALL_NOSPEC
> >
> >
> > It would not explain why it suddenly started to break, unless the clang
> > version also changed, but it would be easy to ascertain and fix (in
> > either KVM or clang).  Dmitry, can you send me the vmx.o and
> > kvm-intel.ko files?
> 
> On a quick glance, Clang does not miscompile this part.

Clang definitely miscompiles the asm, the indirect call operates on the
EFLAGS value, not on @entry as expected.  It looks like clang doesn't honor
ASM_CALL_CONSTRAINT, which effectively tells the compiler that %rsp is
getting clobbered, e.g. the "mov %r14,0x8(%rsp)" is loading @entry for
"callq *0x8(%rsp)", which breaks because of asm's pushes.

clang:

	kvm_before_interrupt(vcpu);

	asm volatile(
ffffffff811b798e:	4c 89 74 24 08       	mov    %r14,0x8(%rsp)
ffffffff811b7993:	48 89 e0             	mov    %rsp,%rax
ffffffff811b7996:	48 83 e4 f0          	and    $0xfffffffffffffff0,%rsp
ffffffff811b799a:	6a 18                	pushq  $0x18
ffffffff811b799c:	50                   	push   %rax
ffffffff811b799d:	9c                   	pushfq 
ffffffff811b799e:	6a 10                	pushq  $0x10
ffffffff811b79a0:	ff 54 24 08          	callq  *0x8(%rsp) <--------- calls the EFLAGS value
kvm_after_interrupt():


gcc:
	kvm_before_interrupt(vcpu);

	asm volatile(
ffffffff8118e17c:	48 89 e0             	mov    %rsp,%rax
ffffffff8118e17f:	48 83 e4 f0          	and    $0xfffffffffffffff0,%rsp
ffffffff8118e183:	6a 18                	pushq  $0x18
ffffffff8118e185:	50                   	push   %rax
ffffffff8118e186:	9c                   	pushfq 
ffffffff8118e187:	6a 10                	pushq  $0x10
ffffffff8118e189:	ff d3                	callq  *%rbx <-------- calls @entry
kvm_after_interrupt():

  reply	other threads:[~2020-03-23 16:39 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-22  6:43 BUG: unable to handle kernel NULL pointer dereference in handle_external_interrupt_irqoff syzbot
2020-03-22  6:59 ` Dmitry Vyukov
2020-03-22  7:03   ` Dmitry Vyukov
2020-03-23  8:18   ` Paolo Bonzini
2020-03-23 16:31     ` Alexander Potapenko
2020-03-23 16:39       ` Sean Christopherson [this message]
2020-03-23 16:43         ` Alexander Potapenko
2020-03-23 16:57         ` Nick Desaulniers
2020-03-23 17:28           ` Nick Desaulniers
2020-03-23 17:55             ` Alexander Potapenko
2020-03-23 18:06               ` Nick Desaulniers
2020-03-23 18:06               ` Alexander Potapenko
2020-03-23 18:16                 ` Nick Desaulniers
2020-03-23 18:49                   ` Nick Desaulniers
2020-03-23 19:12                     ` [PATCH] KVM: VMX: don't allow memory operands for inline asm that modifies SP Nick Desaulniers
2020-03-23 19:30                     ` BUG: unable to handle kernel NULL pointer dereference in handle_external_interrupt_irqoff Nick Desaulniers
2020-03-23 19:39                       ` Paolo Bonzini
2020-03-22  8:53 ` syzbot
2020-03-22 13:29 ` syzbot
2020-03-22 13:43   ` Dmitry Vyukov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200323163925.GP28711@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=bp@alien8.de \
    --cc=clang-built-linux@googlegroups.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=syzbot+3f29ca2efb056a761e38@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.