From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59859) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSWjr-0003as-VC for qemu-devel@nongnu.org; Thu, 20 Aug 2015 16:47:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZSWjq-0002FL-Vy for qemu-devel@nongnu.org; Thu, 20 Aug 2015 16:47:07 -0400 References: <1439942592-28888-1-git-send-email-mreitz@redhat.com> From: Eric Blake Message-ID: <55D63CBD.7090103@redhat.com> Date: Thu, 20 Aug 2015 13:46:53 -0700 MIME-Version: 1.0 In-Reply-To: <1439942592-28888-1-git-send-email-mreitz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="RHQROMoIPwB48aiB1WN3hLCgdXjthXh3b" Subject: Re: [Qemu-devel] [PATCH] qemu-img: Fix crash in amend invocation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-block@nongnu.org Cc: Kevin Wolf , qemu-devel@nongnu.org, qemu-stable@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --RHQROMoIPwB48aiB1WN3hLCgdXjthXh3b Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 08/18/2015 05:03 PM, Max Reitz wrote: > Example: > $ ./qemu-img create -f qcow2 /tmp/t.qcow2 64M > $ ./qemu-img amend -f qcow2 -o backing_file=3D/tmp/t.qcow2, -o help \ > /tmp/t.qcow2 >=20 > This should not crash. This actually is tested by iotest 082, but not > caught due to the segmentation fault being silent (which is something > that needs to be fixed, too). As long as we don't forget to do that, I'm okay with having this patch separate from the testsuite enhancement. However... >=20 > Reported-by: Dr. David Alan Gilbert > Cc: qemu-stable > Signed-off-by: Max Reitz > --- > qemu-img.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) >=20 > diff --git a/qemu-img.c b/qemu-img.c > index 75f4ee4..3ddb391 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -2930,8 +2930,7 @@ static int img_amend(int argc, char **argv) > case 'o': > if (!is_valid_option_list(optarg)) { > error_report("Invalid option list: %s", optarg); > - ret =3D -1; > - goto out; > + return 1; > } > if (!options) { > options =3D g_strdup(optarg); This leaks memory if I call 'qemu-img amend -f qcow2 -o help -o backing_file=3D/tmp/t.qcow2, -o help /tmp/t.qcow2', because it allocates 'options' on the first pass, and only cleans up the allocation in the out label. I think you HAVE to goto out, but fix THAT part of the function to not crash when in this inconsistent state. Looking forward to v2. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --RHQROMoIPwB48aiB1WN3hLCgdXjthXh3b Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJV1jy9AAoJEKeha0olJ0NqtbgH/23xAImswWd0cygUqj1+0Srl ekedICv0+kK9v4d5paUV6YnwQZJspvdbbVE4z0haAIoMxQPc5lsgvRkcJm8m+VXe Z9twipNGVHNki25G4vtniHRct1g9oSfJ/C2qYzhAStIDRh/Oql8mw03SQD5L5PlQ x/pYaa06/QUCV2Sp2t7kjr3JyYznN7sTcr3Qc0mwT4fUzDbJMxEkQPdpFU9RzYbO HhYzOCt0DEWlEYkuAsfSf9s9pvP1c/KICcrV2VqYiqRKaBoMOhCG8KxkBpVOIHHn 8dhsmWtr0eVOrhIP3iY6uWWF2I/LEpi31let+l6d1lcsU10XCpv0w039Y/N7RSE= =zmVS -----END PGP SIGNATURE----- --RHQROMoIPwB48aiB1WN3hLCgdXjthXh3b--