From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1765312AbdAJJnS (ORCPT ); Tue, 10 Jan 2017 04:43:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57054 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753605AbdAJJnQ (ORCPT ); Tue, 10 Jan 2017 04:43:16 -0500 Subject: Re: [PATCH v2 5/6] KVM: x86: prevent setup of invalid routes To: Peter Xu , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= References: <20161216151006.11776-1-rkrcmar@redhat.com> <20161216151006.11776-6-rkrcmar@redhat.com> <20170110052648.GJ4135@pxdev.xzpeter.org> Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org From: Paolo Bonzini Message-ID: Date: Tue, 10 Jan 2017 10:43: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: <20170110052648.GJ4135@pxdev.xzpeter.org> 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.29]); Tue, 10 Jan 2017 09:43:17 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/01/2017 06:26, Peter Xu wrote: > On Fri, Dec 16, 2016 at 04:10:05PM +0100, Radim Krčmář wrote: >> 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 >> Signed-off-by: Radim Krčmář > > Since we are at here, do we need to protect KVM_SET_GSI_ROUTING in > general as well to make sure kernel APIC is there? Skipping the check is harmless. I agree that it should have been always done like you suggest, but right now it may break something. Paolo > > ---------8<--------- > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 482612b..31141a7 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -3057,6 +3057,10 @@ static long kvm_vm_ioctl(struct file *filp, > struct kvm_irq_routing_entry *entries = NULL; > > r = -EFAULT; > + > + if (!irqchip_in_kernel(kvm)) > + goto out; > + > if (copy_from_user(&routing, argp, sizeof(routing))) > goto out; > r = -EINVAL; > > --------->8--------- > > Thanks, > > -- peterx >