| Message ID | 20220701165045.4074471-2-juew@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value | expand |
On Fri, 01 Jul 2022 22:20:45 +0530 Jue Wang <juew@google.com> wrote > Fix an access to vcpu->arch.apic when KVM_X86_SETUP_MCE is called > without KVM_CREATE_IRQCHIP called or KVM_CAP_SPLIT_IRQCHIP is > enabled. > > Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") > Signed-off-by: Jue Wang <juew@google.com> > --- > arch/x86/kvm/x86.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 4322a1365f74..d81020dd0fea 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4820,8 +4820,9 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu, > if (mcg_cap & MCG_CMCI_P) > vcpu->arch.mci_ctl2_banks[bank] = 0; > } > - vcpu->arch.apic->nr_lvt_entries = > - KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); > + if (vcpu->arch.apic) > + vcpu->arch.apic->nr_lvt_entries = > + KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); > > static_call(kvm_x86_setup_mce)(vcpu); > out: > -- > 2.37.0.rc0.161.g10f37bed90-goog > > Hello Jue, There is a syzkaller bug regarding null ptr dereference which is caused by vcpu->arch.apic being NULL, first reported on 27th June. You might want to add it's reported-by line so that it can be marked as fixed. Link: https://syzkaller.appspot.com/bug?id=10b9b238e087a6c9bef2cc48bee2375f58fabbfc I was looking at this bug too and fixed it (i.e. reproducer won't crash) using lapic_in_kernel(vcpu) as a condition instead of null ptr check on vcpu->arch.apic, as it makes more sense to the code reader (the lapic is not there since during kvm_arch_vcpu_create(), it isn't created due to irqchip_in_kernel() check being false). May I suggest that lapic_in_kernel(vcpu) be used instead of the null ptr check? Thanks, Siddh
Hi Siddh, Thanks for the note. I've sent out an updated v2 patch: https://lore.kernel.org/kvm/20220706145957.32156-2-juew@google.com/T/#u Thanks, -Jue On Sun, Jul 3, 2022 at 7:44 AM Siddh Raman Pant <code@siddh.me> wrote: > > On Fri, 01 Jul 2022 22:20:45 +0530 Jue Wang <juew@google.com> wrote > > Fix an access to vcpu->arch.apic when KVM_X86_SETUP_MCE is called > > without KVM_CREATE_IRQCHIP called or KVM_CAP_SPLIT_IRQCHIP is > > enabled. > > > > Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") > > Signed-off-by: Jue Wang <juew@google.com> > > --- > > arch/x86/kvm/x86.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 4322a1365f74..d81020dd0fea 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -4820,8 +4820,9 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu, > > if (mcg_cap & MCG_CMCI_P) > > vcpu->arch.mci_ctl2_banks[bank] = 0; > > } > > - vcpu->arch.apic->nr_lvt_entries = > > - KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); > > + if (vcpu->arch.apic) > > + vcpu->arch.apic->nr_lvt_entries = > > + KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); > > > > static_call(kvm_x86_setup_mce)(vcpu); > > out: > > -- > > 2.37.0.rc0.161.g10f37bed90-goog > > > > > > Hello Jue, > > There is a syzkaller bug regarding null ptr dereference which is caused by > vcpu->arch.apic being NULL, first reported on 27th June. You might want to > add it's reported-by line so that it can be marked as fixed. > > Link: https://syzkaller.appspot.com/bug?id=10b9b238e087a6c9bef2cc48bee2375f58fabbfc > > I was looking at this bug too and fixed it (i.e. reproducer won't crash) > using lapic_in_kernel(vcpu) as a condition instead of null ptr check on > vcpu->arch.apic, as it makes more sense to the code reader (the lapic is > not there since during kvm_arch_vcpu_create(), it isn't created due to > irqchip_in_kernel() check being false). > > May I suggest that lapic_in_kernel(vcpu) be used instead of the null ptr > check? > > Thanks, > Siddh
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 4322a1365f74..d81020dd0fea 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4820,8 +4820,9 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu, if (mcg_cap & MCG_CMCI_P) vcpu->arch.mci_ctl2_banks[bank] = 0; } - vcpu->arch.apic->nr_lvt_entries = - KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); + if (vcpu->arch.apic) + vcpu->arch.apic->nr_lvt_entries = + KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); static_call(kvm_x86_setup_mce)(vcpu); out:
Fix an access to vcpu->arch.apic when KVM_X86_SETUP_MCE is called without KVM_CREATE_IRQCHIP called or KVM_CAP_SPLIT_IRQCHIP is enabled. Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") Signed-off-by: Jue Wang <juew@google.com> --- arch/x86/kvm/x86.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)