On Wed, Jul 10, 2013 at 10:49:56PM +0900, Takuya Yoshikawa wrote: > On Wed, 10 Jul 2013 11:24:39 +0300 > "Michael S. Tsirkin" <[email protected]> wrote: > > > On x86, kvm_arch_create_memslot assumes that rmap/lpage_info for the > > slot are zeroed out: if they weren't, error handling code after out_free > > label will free memory which wasn't allocated here. > > This always happens to be the case because on KVM_MR_DELETE we clear the > > whole arch structure. So there's no bug, but it's cleaner not to rely > > on this here. > > Yes, the assumption is that the function is called only with zero-sized slots. > Since changing the size is not allowed, DELETE-CREATE is the only case we > care about. > > But isn't it possible to make it explicit that zero-sized slots have always > zero-cleared contents instead? Otherwise, there would be many troubles. > Do you have something in mind?
> Takuya > > > > > Make the code more robust by clearing the rmap/lpage_info explicitly. > > > > Signed-off-by: Michael S. Tsirkin <[email protected]> > > --- > > arch/x86/kvm/x86.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index e8ba99c..96e6eb4 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -6922,6 +6922,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot > > *slot, unsigned long npages) > > { > > int i; > > > > + /* Reset in case slot had some rmap/lpage_info. */ > > + memset(&slot->arch.rmap, 0, sizeof slot->arch.rmap); > > + memset(&slot->arch.lpage_info, 0, sizeof slot->arch.lpage_info); > > + > > for (i = 0; i < KVM_NR_PAGE_SIZES; ++i) { > > unsigned long ugfn; > > int lpages; > > -- > > MST > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm" in > > the body of a message to [email protected] > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- > Takuya Yoshikawa <[email protected]> -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
