[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <871r35n6nh.fsf@redhat.com>
Date: Wed, 24 Nov 2021 12:18:42 +0100
From: Vitaly Kuznetsov <vkuznets@...hat.com>
To: pbonzini@...hat.com, David Woodhouse <dwmw@...zon.co.uk>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
syzbot <syzbot+7b7db8bb4db6fd5e157b@...kaller.appspotmail.com>,
syzkaller-bugs@...glegroups.com,
Sean Christopherson <seanjc@...gle.com>
Subject: Re: [syzbot] kernel BUG in kvm_read_guest_offset_cached
syzbot <syzbot+7b7db8bb4db6fd5e157b@...kaller.appspotmail.com> writes:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 4c388a8e740d Merge tag 'zstd-for-linus-5.16-rc1' of git://..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=171ff6eeb00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=6d3b8fd1977c1e73
> dashboard link: https://syzkaller.appspot.com/bug?extid=7b7db8bb4db6fd5e157b
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
>
> Unfortunately, I don't have any reproducer for this issue yet.
No worries, I think I do.
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+7b7db8bb4db6fd5e157b@...kaller.appspotmail.com
>
> ------------[ cut here ]------------
> kernel BUG at arch/x86/kvm/../../../virt/kvm/kvm_main.c:2955!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> CPU: 0 PID: 27639 Comm: syz-executor.0 Not tainted 5.16.0-rc1-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> RIP: 0010:kvm_read_guest_offset_cached+0x3aa/0x440 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2955
> Code: 00 48 c7 c2 c0 08 a2 89 be 0b 03 00 00 48 c7 c7 60 0d a2 89 c6 05 71 f9 73 0c 01 e8 62 19 f8 07 e9 d6 fc ff ff e8 36 1b 6f 00 <0f> 0b e8 2f 1b 6f 00 48 8b 74 24 10 4c 89 ef 4c 89 e1 48 8b 54 24
> RSP: 0018:ffffc9000589fa18 EFLAGS: 00010216
> RAX: 0000000000003b75 RBX: ffff8880722ba798 RCX: ffffc90002b94000
> RDX: 0000000000040000 RSI: ffffffff81087cda RDI: 0000000000000003
> RBP: 0000000000000000 R08: 0000000000000004 R09: ffffc900049dbf53
> R10: ffffffff81087a0f R11: 0000000000000002 R12: 0000000000000004
> R13: ffffc900049d1000 R14: 0000000000000000 R15: ffff8880886c0000
> FS: 00007fd7a562f700(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020000200 CR3: 0000000038e62000 CR4: 00000000003526f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 000000000000c0fe
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> handle_vmptrld arch/x86/kvm/vmx/nested.c:5304 [inline]
The code is:
struct gfn_to_hva_cache *ghc = &vmx->nested.vmcs12_cache;
struct vmcs_hdr hdr;
if (ghc->gpa != vmptr &&
kvm_gfn_to_hva_cache_init(vcpu->kvm, ghc, vmptr, VMCS12_SIZE)) {
...
}
if (kvm_read_guest_offset_cached(vcpu->kvm, ghc, &hdr,
offsetof(struct vmcs12, hdr),
sizeof(hdr))) {
....
It seems that 'nested.vmcs12_cache' is zero-initalized and 'ghc->gpa !=
vmptr' check will pass if VMPTRLD is called with '0' argument.
The following hack to 'state_test.c' can be used to crash host's kernel:
diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
index 32854c1462ad..29b468f1a083 100644
--- a/tools/testing/selftests/kvm/x86_64/state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/state_test.c
@@ -81,6 +81,9 @@ static void vmx_l1_guest_code(struct vmx_pages *vmx_pages)
GUEST_ASSERT(vmx_pages->vmcs_gpa);
GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
GUEST_SYNC(3);
+
+ vmptrld(0);
+
GUEST_ASSERT(load_vmcs(vmx_pages));
GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
This seems to be a regression introduced by
commit cee66664dcd6241a943380ef9dcd2f8a0a7dc47d
Author: David Woodhouse <dwmw@...zon.co.uk>
Date: Mon Nov 15 16:50:26 2021 +0000
KVM: nVMX: Use a gfn_to_hva_cache for vmptrld
Luckily, it's in 5.16-rc2 only.
I think the solution is to assign 'ghc->gpa' to '-1' upon
initialization. I can send a patch if my conclusions seem to be right.
--
Vitaly
Powered by blists - more mailing lists