Re: [PATCH v2 5/6] KVM: x86: prevent setup of invalid routes

[David Hildenbrand]

Am 16.12.2016 um 16:10 schrieb Radim KrÄmÃÅ:
> The check in kvm_set_pic_irq() and kvm_set_ioapic_irq() was just a
> temporary measure until the code improved enough for us to do this.
>
> This changes APIC in a case when KVM_SET_GSI_ROUTING is called to set up pic
> and ioapic routes before KVM_CREATE_IRQCHIP.  Those rules would get overwritten
> by KVM_CREATE_IRQCHIP at best, so it is pointless to allow it.  Userspaces
> hopefully noticed that things don't work if they do that and don't do that.
>
> Reviewed-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Signed-off-by: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
> ---
>  v2: r-b Paolo
> ---
>  arch/x86/kvm/irq_comm.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
> index 1dfeb185a1e3..2639b8d3dce2 100644
> --- a/arch/x86/kvm/irq_comm.c
> +++ b/arch/x86/kvm/irq_comm.c
> @@ -41,15 +41,6 @@ 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_IRQ_LINE.
> -	 */
> -	if (!pic)
> -		return -1;
> -
>  	return kvm_pic_set_irq(pic, e->irqchip.pin, irq_source_id, level);
>  }
>
> @@ -58,10 +49,6 @@ static int kvm_set_ioapic_irq(struct kvm_kernel_irq_routing_entry *e,
>  			      bool line_status)
>  {
>  	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
> -
> -	if (!ioapic)
> -		return -1;
> -
>  	return kvm_ioapic_set_irq(ioapic, e->irqchip.pin, irq_source_id, level,
>  				line_status);
>  }
> @@ -301,10 +288,16 @@ int kvm_set_routing_entry(struct kvm *kvm,
>  			delta = 8;
>  			/* fall through */
>  		case KVM_IRQCHIP_PIC_MASTER:
> +			if (!pic_in_kernel(kvm))
> +				goto out;
> +
>  			e->set = kvm_set_pic_irq;
>  			max_pin = PIC_NUM_PINS;
>  			break;
>  		case KVM_IRQCHIP_IOAPIC:
> +			if (!ioapic_in_kernel(kvm))
> +				goto out;
> +
>  			max_pin = KVM_IOAPIC_NUM_PINS;
>  			e->set = kvm_set_ioapic_irq;
>  			break;

Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>

--

David