From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH] KVM: only set kvm->arch.vioapic when success to init
 ioapic
Date: Mon, 8 Feb 2010 16:07:54 -0200
Message-ID: <20100208180754.GC5415@amt.cnet>
References: <4B6FD2DB.9040104@cn.fujitsu.com>
 <4B6FD377.205@cn.fujitsu.com>
 <4B6FDB2D.9080707@cn.fujitsu.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org
To: Wei Yongjun <yjwei@cn.fujitsu.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:52817 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752231Ab0BHSIc (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 8 Feb 2010 13:08:32 -0500
Content-Disposition: inline
In-Reply-To: <4B6FDB2D.9080707@cn.fujitsu.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Feb 08, 2010 at 05:36:45PM +0800, Wei Yongjun wrote:
> kvm->arch.vioapic is always set either kvm_ioapic_init() is
> success or fail. If kvm_ioapic_init() is fail, the kvm->arch.vioapic
> may point a freed memory.
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> ---
>  virt/kvm/ioapic.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index a2edfd1..e13f529 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -386,7 +386,6 @@ int kvm_ioapic_init(struct kvm *kvm)
>  	if (!ioapic)
>  		return -ENOMEM;
>  	mutex_init(&ioapic->lock);
> -	kvm->arch.vioapic = ioapic;
>  	kvm_ioapic_reset(ioapic);
>  	kvm_iodevice_init(&ioapic->dev, &ioapic_mmio_ops);
>  	ioapic->kvm = kvm;
> @@ -395,6 +394,8 @@ int kvm_ioapic_init(struct kvm *kvm)
>  	mutex_unlock(&kvm->slots_lock);
>  	if (ret < 0)
>  		kfree(ioapic);
> +	else
> +		kvm->arch.vioapic = ioapic;
>  
>  	return ret;
>  }

Same thing as with the PIT issue, kvm->arch.vioapic must be set
before kvm_io_bus_register_dev.

But you're right that kvm->arch.vioapic should be NULLified in case of
failure.