From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58934) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xnuzn-0007EM-Jw for qemu-devel@nongnu.org; Mon, 10 Nov 2014 14:51:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xnuzi-0002aA-JK for qemu-devel@nongnu.org; Mon, 10 Nov 2014 14:51:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60263) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xnuzi-0002Zv-CY for qemu-devel@nongnu.org; Mon, 10 Nov 2014 14:51:22 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAAJpJBB006302 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 10 Nov 2014 14:51:20 -0500 Message-ID: <54611736.4090104@redhat.com> Date: Mon, 10 Nov 2014 12:51:18 -0700 From: Eric Blake MIME-Version: 1.0 References: <1415389165-16157-1-git-send-email-kwolf@redhat.com> <1415389165-16157-8-git-send-email-kwolf@redhat.com> In-Reply-To: <1415389165-16157-8-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="VgXsGfuW1eInObfItqLpOUv526HF8sFau" Subject: Re: [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-devel@nongnu.org Cc: jcody@redhat.com, armbru@redhat.com, stefanha@redhat.com, mreitz@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --VgXsGfuW1eInObfItqLpOUv526HF8sFau Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/07/2014 12:39 PM, Kevin Wolf wrote: > If the user neglects to specify the image format, QEMU probes the > image to guess it automatically, for convenience. >=20 [for those patches in 1-6 where I did not leave comments, I'm happy with them, and saw that Max already gave R-b so I didn't spend thorough review time on them] >=20 > The other differences of this patch to the old one are that it doesn't > silently write something different than the guest requested by zeroing > out some bytes (it fails the request instead) and that it doesn't > maintain a list of signatures in the raw driver (it calls the usual > probe function instead). >=20 > Note that this change doesn't introduce new breakage for false positive= > cases where the guest legitimately writes data into the first sector > that matches the signatures of an image format (e.g. for nested virt): > These cases were broken before, only the failure mode changes from > corruption after the next restart (when the wrong format is probed) to > failing the problematic write request. I would feel better if this commit message explicitly mentioned that the failed write can ONLY occur when probing occurs; therefore, a user can ensure that guests can legitimately write anything to the first sector by explicitly providing a format. But at least the error message does it= =2E I'm still not 100% convinced this is the patch we want, but am happy enough that it won't break libvirt (which strives to always pass a format), so I'm comfortable leaving a review. >=20 > Signed-off-by: Kevin Wolf > --- > block.c | 5 +++-- > block/raw_bsd.c | 57 +++++++++++++++++++++++++++++++++++++++= +++++++- > include/block/block_int.h | 3 +++ > 3 files changed, 62 insertions(+), 3 deletions(-) > @@ -158,6 +202,17 @@ static int raw_open(BlockDriverState *bs, QDict *o= ptions, int flags, > Error **errp) > { > bs->sg =3D bs->file->sg; > + > + if (bs->probed && !bdrv_is_read_only(bs)) { > + fprintf(stderr, > + "WARNING: Image format was not specified for '%s'.\n" > + " Automatically detecting the format is danger= ous for " > + "raw images, write operations on block 0 will be restr= icted.\n" > + " Specify the 'raw' format explicitly to remov= e the " > + "restrictions.\n", This error message works fairly well for me. Maybe the first line could be a bit longer: WARNING: Image format was not specified for '%s', and raw was assumed.\n or maybe: WARNING: Image format was not specified for '%s', and probing guessed raw= =2E\n but even with your original shorter wording, Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --VgXsGfuW1eInObfItqLpOUv526HF8sFau Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJUYRc2AAoJEKeha0olJ0NqgWQH/2rqmGTR8XHeqFvTf7jnL1zB 9gS4LUktcwnOXBy3yzS3CnjEgOhxr0BlKlTp3wZiGz/VkO2xMZaVgnRQkI4Dbg6U 8kHxn9zSCcif2Fa+H4UwJEWVBcdngUv8Cl/G6WrNzcj6BpggDr4rohOMbsRcu4va /SECERRcMmuP1SLP8BArZHB7KHH8pmCk3+ZN5mJR9c7raFmdtHZalQ1LcwMws02n 4xEj09CnGFgS044BY5ro73fvrfVdROwxh9PD/4KrO3gvl+tEGvPrssl1D6scMqoK NrZINCY/7LLpYwNWDe7fswAh+TYOkCwcfiveDV4PlikqKSVm8ttX0EvriO4o/GM= =chDp -----END PGP SIGNATURE----- --VgXsGfuW1eInObfItqLpOUv526HF8sFau--