| Message ID | 20090527164059.23966.75880.stgit@dev.haskins.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Gregory Haskins (ghaskins@novell.com) wrote: > It would appear that we are invoking kfree() on the wrong pointer in the > destructor for the coalesced_mmio device. This would result in a potential > leak during shutdown. Happens to work and not leak: struct kvm_coalesced_mmio_dev { struct kvm_io_device dev; struct kvm *kvm; int nb_zones; struct kvm_coalesced_mmio_zone zone[KVM_COALESCED_MMIO_ZONE_MAX]; }; > Signed-off-by: Gregory Haskins <ghaskins@novell.com> > --- > > virt/kvm/coalesced_mmio.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c > index 5ae620d..03ea280 100644 > --- a/virt/kvm/coalesced_mmio.c > +++ b/virt/kvm/coalesced_mmio.c > @@ -80,7 +80,10 @@ static void coalesced_mmio_write(struct kvm_io_device *this, > > static void coalesced_mmio_destructor(struct kvm_io_device *this) > { > - kfree(this); > + struct kvm_coalesced_mmio_dev *dev = > + (struct kvm_coalesced_mmio_dev *)this->private; I think container_of() makes more sense here. thanks, -chris -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Chris Wright wrote: > * Gregory Haskins (ghaskins@novell.com) wrote: > >> It would appear that we are invoking kfree() on the wrong pointer in the >> destructor for the coalesced_mmio device. This would result in a potential >> leak during shutdown. >> > > Happens to work and not leak: > > struct kvm_coalesced_mmio_dev { > struct kvm_io_device dev; > struct kvm *kvm; > int nb_zones; > struct kvm_coalesced_mmio_zone zone[KVM_COALESCED_MMIO_ZONE_MAX]; > }; > > Ah, yes. That explains it. Still sloppy, tho. >> Signed-off-by: Gregory Haskins <ghaskins@novell.com> >> --- >> >> virt/kvm/coalesced_mmio.c | 5 ++++- >> 1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c >> index 5ae620d..03ea280 100644 >> --- a/virt/kvm/coalesced_mmio.c >> +++ b/virt/kvm/coalesced_mmio.c >> @@ -80,7 +80,10 @@ static void coalesced_mmio_write(struct kvm_io_device *this, >> >> static void coalesced_mmio_destructor(struct kvm_io_device *this) >> { >> - kfree(this); >> + struct kvm_coalesced_mmio_dev *dev = >> + (struct kvm_coalesced_mmio_dev *)this->private; >> > > I think container_of() makes more sense here. > I was working on that patch when I noticed the "leak" above. Figured I should send the fix out first, in case my container_of patch is shot down. Just polishing it up now. Will send out soon. -Greg
* Gregory Haskins (gregory.haskins@gmail.com) wrote: > Chris Wright wrote: > > * Gregory Haskins (ghaskins@novell.com) wrote: > >> It would appear that we are invoking kfree() on the wrong pointer in the > >> destructor for the coalesced_mmio device. This would result in a potential > >> leak during shutdown. > > > > Happens to work and not leak: > > > > struct kvm_coalesced_mmio_dev { > > struct kvm_io_device dev; > > struct kvm *kvm; > > int nb_zones; > > struct kvm_coalesced_mmio_zone zone[KVM_COALESCED_MMIO_ZONE_MAX]; > > }; > > > > > Ah, yes. That explains it. Still sloppy, tho. Definitely. > >> static void coalesced_mmio_destructor(struct kvm_io_device *this) > >> { > >> - kfree(this); > >> + struct kvm_coalesced_mmio_dev *dev = > >> + (struct kvm_coalesced_mmio_dev *)this->private; > >> > > > > I think container_of() makes more sense here. > > I was working on that patch when I noticed the "leak" above. Figured I > should send the fix out first, in case my container_of patch is shot down. > > Just polishing it up now. Will send out soon. Sounds good. thanks, -chris -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c index 5ae620d..03ea280 100644 --- a/virt/kvm/coalesced_mmio.c +++ b/virt/kvm/coalesced_mmio.c @@ -80,7 +80,10 @@ static void coalesced_mmio_write(struct kvm_io_device *this, static void coalesced_mmio_destructor(struct kvm_io_device *this) { - kfree(this); + struct kvm_coalesced_mmio_dev *dev = + (struct kvm_coalesced_mmio_dev *)this->private; + + kfree(dev); } int kvm_coalesced_mmio_init(struct kvm *kvm)
It would appear that we are invoking kfree() on the wrong pointer in the destructor for the coalesced_mmio device. This would result in a potential leak during shutdown. Signed-off-by: Gregory Haskins <ghaskins@novell.com> --- virt/kvm/coalesced_mmio.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html