From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gregory Haskins Subject: Re: [PATCH] kvm: fix coalesced_mmio leak on shutdown Date: Wed, 27 May 2009 16:32:33 -0400 Message-ID: <4A1DA361.6050303@gmail.com> References: <20090527164059.23966.75880.stgit@dev.haskins.net> <20090527202932.GI20823@sequoia.sous-sol.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig95C7059E41369ACC8285FEB9" Cc: Gregory Haskins , avi@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org To: Chris Wright Return-path: Received: from mail-gx0-f166.google.com ([209.85.217.166]:47070 "EHLO mail-gx0-f166.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755976AbZE0Uck (ORCPT ); Wed, 27 May 2009 16:32:40 -0400 In-Reply-To: <20090527202932.GI20823@sequoia.sous-sol.org> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig95C7059E41369ACC8285FEB9 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Chris Wright wrote: > * Gregory Haskins (ghaskins@novell.com) wrote: > =20 >> It would appear that we are invoking kfree() on the wrong pointer in t= he >> destructor for the coalesced_mmio device. This would result in a pote= ntial >> leak during shutdown. >> =20 > > Happens to work and not leak: > > struct kvm_coalesced_mmio_dev {=20 > struct kvm_io_device dev; > struct kvm *kvm; > int nb_zones; > struct kvm_coalesced_mmio_zone zone[KVM_COALESCED_MMIO_ZONE_MAX= ]; > }; > > =20 Ah, yes. That explains it. Still sloppy, tho. >> Signed-off-by: Gregory Haskins >> --- >> >> 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_devi= ce *this, >> =20 >> static void coalesced_mmio_destructor(struct kvm_io_device *this) >> { >> - kfree(this); >> + struct kvm_coalesced_mmio_dev *dev =3D >> + (struct kvm_coalesced_mmio_dev *)this->private; >> =20 > > I think container_of() makes more sense here. > =20 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= =2E Just polishing it up now. Will send out soon. -Greg --------------enig95C7059E41369ACC8285FEB9 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.11 (Darwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkodo2UACgkQP5K2CMvXmqGG5wCeL+jDLYgWyEBEn97niW5bu6kt JnEAn2pqNscKSB+uJ2jO96rmSfcNRJkv =tjio -----END PGP SIGNATURE----- --------------enig95C7059E41369ACC8285FEB9--