Skip to contentSkip to conversation

kvm: fix coalesced_mmio leak on shutdown

PATCH Superseded Gregory Haskins 3 participants 4 messages May 27, 2009 KVM

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kvm: fix coalesced_mmio leak on shutdown

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.

---

 virt/kvm/coalesced_mmio.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
virt/kvm/coalesced_mmio.c +4 -1
index 5ae620d..03ea280 100644
@@ -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)
#1 lore raw
Quoted 3 lines · * Gregory Haskins ([email protected]) 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];
};
Quoted 11 lines

---

 virt/kvm/coalesced_mmio.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
virt/kvm/coalesced_mmio.c +0 -0
index 5ae620d..03ea280 100644

virt/kvm/coalesced_mmio.c static void coalesced_mmio_write(struct kvm_io_device *this, · line 84

Quoted 6 lines
virt/kvm/coalesced_mmio.c +2 -1
@@ -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

#2 lore raw
Quoted 7 lines · Chris Wright
Quoted 4 lines · * Gregory Haskins ([email protected]) 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.

Quoted 10 lines

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.

Quoted 12 lines
Quoted 12 lines

---

 virt/kvm/coalesced_mmio.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
virt/kvm/coalesced_mmio.c +0 -0
index 5ae620d..03ea280 100644
@@ -80,7 +80,10 @@ static void coalesced_mmio_write(struct kvm_io_device *this,
 
Quoted 9 lines

virt/kvm/coalesced_mmio.c static void coalesced_mmio_write(struct kvm_io_device *this, · line 88

Quoted 6 lines
virt/kvm/coalesced_mmio.c +2 -1
 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

#3 lore raw
Quoted 16 lines · * Gregory Haskins ([email protected]) wrote:
Quoted 5 lines · Chris Wright
Quoted 3 lines · * Gregory Haskins ([email protected]) 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.

Quoted 10 lines

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];
};
Quoted 1 line

Ah, yes. That explains it. Still sloppy, tho.

Definitely.

Quoted 9 lines
Quoted 8 lines

virt/kvm/coalesced_mmio.c static void coalesced_mmio_write(struct kvm_io_device *this, · line 84

Quoted 6 lines

static void coalesced_mmio_destructor(struct kvm_io_device *this) {

virt/kvm/coalesced_mmio.c +2 -1
-	kfree(this);
+	struct kvm_coalesced_mmio_dev *dev =
+		(struct kvm_coalesced_mmio_dev *)this->private;

I think container_of() makes more sense here.

Quoted 4 lines

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