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

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <461963f5-fddf-5b98-da01-0adb12212029@redhat.com>
Date:   Tue, 3 Jan 2017 14:06:34 +0100
From:   David Hildenbrand <david@...hat.com>
To:     Radim Krčmář <rkrcmar@...hat.com>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc:     Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v2 5/6] KVM: x86: prevent setup of invalid routes

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@...hat.com>
> Signed-off-by: Radim Krčmář <rkrcmar@...hat.com>
> ---
>  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@...hat.com>

-- 

David