linux-kernel - Re: [PATCH] KVM: x86: check for pic and ioapic presence before use

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANRm+CwMTncCvFcZNOSDc6aXiOOzg-rVSM_0MKb1vJ-0wBTENA@mail.gmail.com>
Date:   Thu, 22 Dec 2016 17:56:28 +0800
From:   Wanpeng Li <kernellwp@...il.com>
To:     Radim Krčmář <rkrcmar@...hat.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        kvm <kvm@...r.kernel.org>, stable@...r.kernel.org,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Steve Rutherford <srutherford@...gle.com>
Subject: Re: [PATCH] KVM: x86: check for pic and ioapic presence before use

2016-12-21 20:44 GMT+08:00 Radim Krčmář <rkrcmar@...hat.com>:
> 2016-12-20 19:59+0800, Wanpeng Li:
>> 2016-11-24 20:42 GMT+08:00 Radim Krčmář <rkrcmar@...hat.com>:
>>> 2016-11-23 22:58+0100, Paolo Bonzini:
>>>> On 23/11/2016 21:25, Radim Krčmář wrote:
>>>>> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
>>>>> index 25810b144b58..ddd63b8b176e 100644
>>>>> --- a/arch/x86/kvm/irq_comm.c
>>>>> +++ b/arch/x86/kvm/irq_comm.c
>>>>> @@ -41,6 +41,15 @@ static int kvm_set_pic_irq(struct kvm_kernel_irq_routing_entry *e,
>>>>>                         bool line_status)
>>>>>  {
>>>>>      struct kvm_pic *pic = pic_irqchip(kvm);
>>>>> +
>>>>> +    /*
>>>>> +     * XXX: rejecting pic routes when pic isn't in use would be better,
>>>>> +     * but the default routing table is installed while kvm->arch.vpic is
>>>>> +     * NULL and KVM_CREATE_IRQCHIP can race with KVM_SET_GSI_ROUTING.
>>>>> +     */
>>>>> +    if (!pic)
>>>>> +            return -1;
>>>>> +
>>>>>      return kvm_pic_set_irq(pic, e->irqchip.pin, irq_source_id, level);
>>>>>  }
>>>>>
>>>>
>>>> Can you explain the race with the default routing table better?  It
>>>> seems to me that it can only make the routing table go from invalid to
>>>> valid (there is no KVM_DESTROY_IRQCHIP) so it's benign.
>>>
>>> Oops, I wrote the race with wrong IOCTL -- it should be KVM_IRQ_LINE.
>>>
>>>  1) set KVM_CAP_SPLIT_IRQCHIP (unlocks KVM_IRQ_LINE)
>>>  a) call KVM_CREATE_IRQCHIP (creates routes while !kvm->arch.vpic)
>>>  b) concurrently call KVM_IRQ_LINE for PIO routes (dereferences NULL)
>>
>> If we should not go through irqfd if irqchip is split?
>
> I also remember hearing about that -- do you remember where it was?

Not sure. :)

>
> The documentation does not say that and irqfd is mostly optimization for
> KVM_IRQ_LINE ... QEMU uses KVM_IRQ_LINE_STATUS with split irqchip, so we
> can't easily say the opposite now.

How irqfd optimizes KVM_IRQ_LINE_STATUS? I didn't observe that they
have relationship.

Regards,
Wanpeng Li