From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juan Quintela Subject: Re: KVM: properly check max PIC pin in irq route setup Date: Tue, 12 Jan 2010 20:45:35 +0100 Message-ID: References: <20100112184209.GA4055@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm , Avi Kivity , Gleb Natapov To: Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:1025 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754274Ab0ALTpp (ORCPT ); Tue, 12 Jan 2010 14:45:45 -0500 In-Reply-To: <20100112184209.GA4055@amt.cnet> (Marcelo Tosatti's message of "Tue, 12 Jan 2010 16:42:09 -0200") Sender: kvm-owner@vger.kernel.org List-ID: Marcelo Tosatti wrote: > Otherwise memory beyond irq_states[16] might be accessed. > > Noticed by Juan Quintela. > > Signed-off-by: Marcelo Tosatti > > diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c > index 9b07734..9fd5b3e 100644 > --- a/virt/kvm/irq_comm.c > +++ b/virt/kvm/irq_comm.c > @@ -302,6 +302,7 @@ static int setup_routing_entry(struct kvm_irq_routing_table *rt, > { > int r = -EINVAL; > int delta; > + unsigned max_pin; > struct kvm_kernel_irq_routing_entry *ei; > struct hlist_node *n; > > @@ -322,12 +323,15 @@ static int setup_routing_entry(struct kvm_irq_routing_table *rt, > switch (ue->u.irqchip.irqchip) { > case KVM_IRQCHIP_PIC_MASTER: > e->set = kvm_set_pic_irq; > + max_pin = 16; once you are here, you can use the constant name: PIC_NUM_PINS Humm, that is not going to fly, it is defined in x86 code, and it is used in generic code. No great ideas here :( > break; > case KVM_IRQCHIP_PIC_SLAVE: > e->set = kvm_set_pic_irq; > + max_pin = 16; > delta = 8; > break; > case KVM_IRQCHIP_IOAPIC: > + max_pin = KVM_IOAPIC_NUM_PINS; > e->set = kvm_set_ioapic_irq; > break; > default: > @@ -335,7 +339,7 @@ static int setup_routing_entry(struct kvm_irq_routing_table *rt, > } > e->irqchip.irqchip = ue->u.irqchip.irqchip; > e->irqchip.pin = ue->u.irqchip.pin + delta; > - if (e->irqchip.pin >= KVM_IOAPIC_NUM_PINS) > + if (e->irqchip.pin >= max_pin) > goto out; > rt->chip[ue->u.irqchip.irqchip][e->irqchip.pin] = ue->gsi; > break; For the rest, patch fixes my concerns. Acked-by: Juan Quintela