From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932684AbdABKJT (ORCPT ); Mon, 2 Jan 2017 05:09:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50328 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754802AbdABKJR (ORCPT ); Mon, 2 Jan 2017 05:09:17 -0500 Subject: Re: [PATCH] KVM: ioapic: fix NULL deref ioapic->lock To: Wanpeng Li , linux-kernel@vger.kernel.org, kvm@vger.kernel.org References: <1483242289-12323-1-git-send-email-wanpeng.li@hotmail.com> Cc: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Dmitry Vyukov , Wanpeng Li From: Paolo Bonzini Message-ID: Date: Mon, 2 Jan 2017 11:09:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <1483242289-12323-1-git-send-email-wanpeng.li@hotmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 02 Jan 2017 10:09:18 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/01/2017 04:44, Wanpeng Li wrote: > From: Wanpeng Li > > This was reported by syzkaller: > > BUG: unable to handle kernel NULL pointer dereference at 00000000000001b0 > IP: _raw_spin_lock+0xc/0x30 > PGD 3e28eb067 > PUD 3f0ac6067 > PMD 0 > Oops: 0002 [#1] SMP > CPU: 0 PID: 2431 Comm: test Tainted: G OE 4.10.0-rc1+ #3 > Call Trace: > ? kvm_ioapic_scan_entry+0x3e/0x110 [kvm] > kvm_arch_vcpu_ioctl_run+0x10a8/0x15f0 [kvm] > ? pick_next_task_fair+0xe1/0x4e0 > ? kvm_arch_vcpu_load+0xea/0x260 [kvm] > kvm_vcpu_ioctl+0x33a/0x600 [kvm] > ? hrtimer_try_to_cancel+0x29/0x130 > ? do_nanosleep+0x97/0xf0 > do_vfs_ioctl+0xa1/0x5d0 > ? __hrtimer_init+0x90/0x90 > ? do_nanosleep+0x5b/0xf0 > SyS_ioctl+0x79/0x90 > do_syscall_64+0x6e/0x180 > entry_SYSCALL64_slow_path+0x25/0x25 > RIP: _raw_spin_lock+0xc/0x30 RSP: ffffa43688973cc0 > > KVM will skip over create pic/ioapic if there is a created vCPU. However, > there is no guarantee whether ioapic is present when rescan ioapic which > results in NULL dereference ioapic->lock. This patch fix it by adding the > ioapic present check to ioapic scan. > > Reported-by: Dmitry Vyukov > Cc: Paolo Bonzini > Cc: Radim Krčmář > Signed-off-by: Wanpeng Li > --- > arch/x86/kvm/x86.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 51ccfe0..9ca175c 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6556,7 +6556,8 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu) > else { > if (vcpu->arch.apicv_active) > kvm_x86_ops->sync_pir_to_irr(vcpu); > - kvm_ioapic_scan_entry(vcpu, vcpu->arch.ioapic_handled_vectors); > + if (ioapic_irqchip(vcpu->kvm)) > + kvm_ioapic_scan_entry(vcpu, vcpu->arch.ioapic_handled_vectors); > } > bitmap_or((ulong *)eoi_exit_bitmap, vcpu->arch.ioapic_handled_vectors, > vcpu_to_synic(vcpu)->vec_bitmap, 256); Commit message for fuzzing bugs have usually included a beautified reproducer. However, you are not even saying if it is a race, or it is deterministic. The fix seems wrong to me at first impression, because "LAPIC enabled" and "irqchip not split" should imply the existence of an in-kernel IOAPIC. However, I cannot suggest the right course of action without seeing a testcase. Paolo