From: Michal Luczaj <mhal@rbox.co>
To: Sean Christopherson <seanjc@google.com>
Cc: kvm@vger.kernel.org, pbonzini@redhat.com
Subject: Re: [PATCH 8/8] KVM: x86: Fix NULL pointer dereference in kvm_xen_set_evtchn_fast()
Date: Thu, 20 Oct 2022 17:59:20 +0200 [thread overview]
Message-ID: <0574dd3d-4272-fc93-50c0-ba2994e272ba@rbox.co> (raw)
In-Reply-To: <Y0h0/x3Fvn17zVt6@google.com>
On 10/13/22 22:28, Sean Christopherson wrote:
> On Thu, Oct 13, 2022, Sean Christopherson wrote:
>> On Mon, Oct 10, 2022, Sean Christopherson wrote:
>>> On Wed, Sep 21, 2022, Michal Luczaj wrote:
>>> If this fixes things on your end (I'll properly test tomorrow too), I'll post a
>>> v2 of the entire series. There are some cleanups that can be done on top, e.g.
>>> I think we should drop kvm_gpc_unmap() entirely until there's actually a user,
>>> because it's not at all obvious that it's (a) necessary and (b) has desirable
>>> behavior.
>>
>> Sorry for the delay, I initially missed that you included a selftest for the race
>> in the original RFC. The kernel is no longer exploding, but the test is intermittently
>> soft hanging waiting for the "IRQ". I'll debug and hopefully post tomorrow.
>
> Ended up being a test bug (technically). KVM drops the timer IRQ if the shared
> info page is invalid. As annoying as that is, there's isn't really a better
> option, and invalidating a shared page while vCPUs are running really is a VMM
> bug.
>
> To fix, I added an intermediate stage in the test that re-arms the timer if the
> IRQ doesn't arrive in a reasonable amount of time.
>
> Patches incoming...
Sorry for the late reply, I was away.
Thank you for the whole v2 series. And I'm glad you've found my testcase
useful, even if a bit buggy ;)
Speaking about SCHEDOP_poll, are XEN vmcalls considered trusted?
I've noticed that kvm_xen_schedop_poll() fetches guest-provided
sched_poll.ports without checking if the values are sane. Then, in
wait_pending_event(), there's test_bit(ports[i], pending_bits) which
(for some high ports[i] values) results in KASAN complaining about
"use-after-free":
[ 36.463417] ==================================================================
[ 36.463564] BUG: KASAN: use-after-free in kvm_xen_hypercall+0xf39/0x1110 [kvm]
[ 36.463962] Read of size 8 at addr ffff88815b87b800 by task xen_shinfo_test/956
[ 36.464149] CPU: 1 PID: 956 Comm: xen_shinfo_test Not tainted 6.1.0-rc1-kasan+ #49
[ 36.464252] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.0-3-3 04/01/2014
[ 36.464259] Call Trace:
[ 36.464259] <TASK>
[ 36.464259] dump_stack_lvl+0x5b/0x73
[ 36.464259] print_report+0x17f/0x477
[ 36.464259] ? __virt_addr_valid+0xd5/0x150
[ 36.464259] ? kvm_xen_hypercall+0xf39/0x1110 [kvm]
[ 36.464259] ? kvm_xen_hypercall+0xf39/0x1110 [kvm]
[ 36.464259] kasan_report+0xbb/0xf0
[ 36.464259] ? kvm_xen_hypercall+0xf39/0x1110 [kvm]
[ 36.464259] kasan_check_range+0x136/0x1b0
[ 36.464259] kvm_xen_hypercall+0xf39/0x1110 [kvm]
[ 36.464259] ? kvm_xen_set_evtchn.part.0+0x190/0x190 [kvm]
[ 36.464259] ? get_kvmclock+0x86/0x360 [kvm]
[ 36.464259] ? pvclock_clocksource_read+0x13a/0x190
[ 36.464259] kvm_emulate_hypercall+0x1d7/0x860 [kvm]
[ 36.464259] ? get_kvmclock+0x151/0x360 [kvm]
[ 36.464259] ? kvm_fast_pio+0x260/0x260 [kvm]
[ 36.464259] ? kvm_post_set_cr4+0xf0/0xf0 [kvm]
[ 36.464259] ? lock_release+0x9c/0x430
[ 36.464259] ? rcu_qs+0x2b/0xb0
[ 36.464259] ? rcu_note_context_switch+0x18e/0x9b0
[ 36.464259] ? rcu_read_lock_sched_held+0x10/0x70
[ 36.464259] ? lock_acquire+0xb1/0x3d0
[ 36.464259] ? vmx_vmexit+0x6c/0x19d [kvm_intel]
[ 36.464259] ? vmx_vmexit+0x8d/0x19d [kvm_intel]
[ 36.464259] ? rcu_read_lock_sched_held+0x10/0x70
[ 36.464259] ? lock_acquire+0xb1/0x3d0
[ 36.464259] ? lock_downgrade+0x380/0x380
[ 36.464259] ? vmx_vcpu_run+0x5bf/0x1260 [kvm_intel]
[ 36.464259] vmx_handle_exit+0x295/0xa50 [kvm_intel]
[ 36.464259] vcpu_enter_guest.constprop.0+0x1436/0x1ed0 [kvm]
[ 36.464259] ? kvm_check_and_inject_events+0x800/0x800 [kvm]
[ 36.464259] ? lock_downgrade+0x380/0x380
[ 36.464259] ? __blkcg_punt_bio_submit+0xd0/0xd0
[ 36.464259] ? kvm_arch_vcpu_ioctl_run+0xa46/0xf70 [kvm]
[ 36.464259] ? unlock_page_memcg+0x1e0/0x1e0
[ 36.464259] ? __local_bh_enable_ip+0x8f/0x100
[ 36.464259] ? trace_hardirqs_on+0x2d/0xf0
[ 36.464259] ? fpu_swap_kvm_fpstate+0xbd/0x1c0
[ 36.464259] ? kvm_arch_vcpu_ioctl_run+0x418/0xf70 [kvm]
[ 36.464259] kvm_arch_vcpu_ioctl_run+0x418/0xf70 [kvm]
[ 36.464259] kvm_vcpu_ioctl+0x332/0x8f0 [kvm]
[ 36.464259] ? kvm_clear_dirty_log_protect+0x430/0x430 [kvm]
[ 36.464259] ? do_vfs_ioctl+0x951/0xbf0
[ 36.464259] ? vfs_fileattr_set+0x480/0x480
[ 36.464259] ? kernel_write+0x360/0x360
[ 36.464259] ? selinux_inode_getsecctx+0x50/0x50
[ 36.464259] ? ioctl_has_perm.constprop.0.isra.0+0x133/0x200
[ 36.464259] ? selinux_inode_getsecctx+0x50/0x50
[ 36.464259] ? ksys_write+0xc4/0x140
[ 36.464259] ? __ia32_sys_read+0x40/0x40
[ 36.464259] ? lockdep_hardirqs_on_prepare+0xe/0x220
[ 36.464259] __x64_sys_ioctl+0xb8/0xf0
[ 36.464259] do_syscall_64+0x55/0x80
[ 36.464259] ? lockdep_hardirqs_on_prepare+0xe/0x220
[ 36.464259] entry_SYSCALL_64_after_hwframe+0x46/0xb0
[ 36.464259] RIP: 0033:0x7f81e303ec6b
[ 36.464259] Code: 73 01 c3 48 8b 0d b5 b1 1b 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 85 b1 1b 00 f7 d8 64 89 01 48
[ 36.464259] RSP: 002b:00007fff72009028 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 36.464259] RAX: ffffffffffffffda RBX: 00007f81e33936c0 RCX: 00007f81e303ec6b
[ 36.464259] RDX: 0000000000000000 RSI: 000000000000ae80 RDI: 0000000000000007
[ 36.464259] RBP: 00000000008457b0 R08: 0000000000000000 R09: 00000000004029f6
[ 36.464259] R10: 00007f81e31b838b R11: 0000000000000246 R12: 00007f81e33a4000
[ 36.464259] R13: 00007f81e33a2000 R14: 0000000000000000 R15: 00007f81e33a3020
[ 36.464259] </TASK>
[ 36.464259] The buggy address belongs to the physical page:
[ 36.464259] page:00000000d9b176a3 refcount:0 mapcount:-128 mapping:0000000000000000 index:0x1 pfn:0x15b87b
[ 36.464259] flags: 0x17ffffc0000000(node=0|zone=2|lastcpupid=0x1fffff)
[ 36.464259] raw: 0017ffffc0000000 ffffea0005504e48 ffffea00056cf688 0000000000000000
[ 36.464259] raw: 0000000000000001 0000000000000000 00000000ffffff7f 0000000000000000
[ 36.464259] page dumped because: kasan: bad access detected
[ 36.464259] Memory state around the buggy address:
[ 36.464259] ffff88815b87b700: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 36.464259] ffff88815b87b780: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 36.464259] >ffff88815b87b800: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 36.464259] ^
[ 36.464259] ffff88815b87b880: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 36.464259] ffff88815b87b900: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 36.464259] ==================================================================
I can't reproduce it under non-KASAN build, I'm not sure what's going on.
Anyway, here's the testcase, applies after your selftests patches in
https://lore.kernel.org/kvm/20221013211234.1318131-1-seanjc@google.com/
---
.../selftests/kvm/x86_64/xen_shinfo_test.c | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
index 2a5727188c8d..402e3d7b86b0 100644
--- a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
@@ -375,6 +375,29 @@ static void guest_code(void)
guest_saw_irq = false;
GUEST_SYNC(24);
+ /* Terminate racer thread */
+
+ GUEST_SYNC(25);
+ /* Test SCHEDOP_poll out-of-bounds read */
+
+ p = (struct sched_poll) {
+ .ports = ports,
+ .nr_ports = 1,
+ .timeout = 1
+ };
+
+ ports[0] = (PAGE_SIZE*2) << 3;
+ for (i = 0; i < 0x1000; i++) {
+ asm volatile("vmcall"
+ : "=a" (rax)
+ : "a" (__HYPERVISOR_sched_op),
+ "D" (SCHEDOP_poll),
+ "S" (&p)
+ : "memory");
+ ports[0] += PAGE_SIZE << 3;
+ }
+
+ GUEST_SYNC(26);
}
static int cmp_timespec(struct timespec *a, struct timespec *b)
@@ -925,6 +948,21 @@ int main(int argc, char *argv[])
ret = pthread_join(thread, 0);
TEST_ASSERT(ret == 0, "pthread_join() failed: %s", strerror(ret));
+
+ /* shinfo juggling done; reset to a valid GFN. */
+ struct kvm_xen_hvm_attr ha = {
+ .type = KVM_XEN_ATTR_TYPE_SHARED_INFO,
+ .u.shared_info.gfn = SHINFO_REGION_GPA / PAGE_SIZE,
+ };
+ vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &ha);
+ break;
+
+ case 25:
+ if (verbose)
+ printf("Testing SCHEDOP_poll out-of-bounds read\n");
+ break;
+
+ case 26:
goto done;
case 0x20:
--
2.38.0
next prev parent reply other threads:[~2022-10-20 15:59 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-16 0:54 [RFC PATCH 0/4] KVM: x86/xen: shinfo cache lock corruption Michal Luczaj
2022-09-16 0:54 ` [RFC PATCH 1/4] KVM: x86/xen: Ensure kvm_xen_set_evtchn_fast() can use shinfo_cache Michal Luczaj
2022-09-16 0:54 ` [RFC PATCH 2/4] KVM: x86/xen: Ensure kvm_xen_schedop_poll() " Michal Luczaj
2022-09-16 0:54 ` [RFC PATCH 3/4] KVM: x86/xen: Disallow gpc locks reinitialization Michal Luczaj
2022-09-16 17:12 ` Sean Christopherson
2022-09-18 23:13 ` Michal Luczaj
2022-09-21 2:01 ` [PATCH 0/8] KVM: x86: gfn_to_pfn_cache cleanups and a fix Michal Luczaj
2022-09-21 2:01 ` [PATCH 1/8] KVM: x86: Add initializer for gfn_to_pfn_cache Michal Luczaj
2022-10-10 23:38 ` Sean Christopherson
2022-09-21 2:01 ` [PATCH 2/8] KVM: x86: Shorten gfn_to_pfn_cache function names Michal Luczaj
2022-09-21 2:01 ` [PATCH 3/8] KVM: x86: Remove unused argument in gpc_unmap_khva() Michal Luczaj
2022-09-21 2:01 ` [PATCH 4/8] KVM: x86: Store immutable gfn_to_pfn_cache properties Michal Luczaj
2022-10-10 23:42 ` Sean Christopherson
2022-10-11 0:37 ` Sean Christopherson
2022-09-21 2:01 ` [PATCH 5/8] KVM: x86: Clean up kvm_gpc_check() Michal Luczaj
2022-09-21 2:01 ` [PATCH 6/8] KVM: x86: Clean up hva_to_pfn_retry() Michal Luczaj
2022-09-21 2:01 ` [PATCH 7/8] KVM: x86: Clean up kvm_gpc_refresh(), kvm_gpc_unmap() Michal Luczaj
2022-09-21 2:01 ` [PATCH 8/8] KVM: x86: Fix NULL pointer dereference in kvm_xen_set_evtchn_fast() Michal Luczaj
2022-10-10 23:28 ` Sean Christopherson
2022-10-13 0:22 ` Sean Christopherson
2022-10-13 20:28 ` Sean Christopherson
2022-10-20 15:59 ` Michal Luczaj [this message]
2022-10-20 16:58 ` Sean Christopherson
2022-10-21 2:39 ` Michal Luczaj
2022-10-05 12:30 ` [PATCH v2 0/8] KVM: x86: gfn_to_pfn_cache cleanups and a fix Michal Luczaj
2022-10-05 12:30 ` [PATCH v2 1/8] KVM: x86: Add initializer for gfn_to_pfn_cache Michal Luczaj
2022-10-05 12:30 ` [PATCH v2 2/8] KVM: x86: Shorten gfn_to_pfn_cache function names Michal Luczaj
2022-10-05 12:30 ` [PATCH v2 3/8] KVM: x86: Remove unused argument in gpc_unmap_khva() Michal Luczaj
2022-10-05 12:30 ` [PATCH v2 4/8] KVM: x86: Store immutable gfn_to_pfn_cache properties Michal Luczaj
2022-10-05 12:30 ` [PATCH v2 5/8] KVM: x86: Clean up kvm_gpc_check() Michal Luczaj
2022-10-05 12:30 ` [PATCH v2 6/8] KVM: x86: Clean up hva_to_pfn_retry() Michal Luczaj
2022-10-05 12:30 ` [PATCH v2 7/8] KVM: x86: Clean up kvm_gpc_refresh(), kvm_gpc_unmap() Michal Luczaj
2022-10-05 12:30 ` [PATCH v2 8/8] KVM: x86: Fix NULL pointer dereference in kvm_xen_set_evtchn_fast() Michal Luczaj
2022-09-16 0:54 ` [RFC PATCH 4/4] KVM: x86/xen: Test shinfo_cache lock races Michal Luczaj
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=0574dd3d-4272-fc93-50c0-ba2994e272ba@rbox.co \
--to=mhal@rbox.co \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).