From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LKbgk-0001zU-6q for qemu-devel@nongnu.org; Wed, 07 Jan 2009 11:55:26 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LKbgj-0001z2-ET for qemu-devel@nongnu.org; Wed, 07 Jan 2009 11:55:25 -0500 Received: from [199.232.76.173] (port=60645 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LKbgj-0001yw-9p for qemu-devel@nongnu.org; Wed, 07 Jan 2009 11:55:25 -0500 Received: from qw-out-1920.google.com ([74.125.92.144]:20691) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LKbgi-0007cb-Qg for qemu-devel@nongnu.org; Wed, 07 Jan 2009 11:55:25 -0500 Received: by qw-out-1920.google.com with SMTP id 5so3857470qwc.4 for ; Wed, 07 Jan 2009 08:55:23 -0800 (PST) Message-ID: <4964DE75.1010502@codemonkey.ws> Date: Wed, 07 Jan 2009 10:55:17 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] qemu: block.c: introducing "fmt:FMT:" prefix to image-filenames References: <49616228.4030608@redhat.com> In-Reply-To: <49616228.4030608@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Shahar Frank , Uri Lublin Uri Lublin wrote: > Hello, > > This patch below can be considered as a version 2 of Shahar's "Qemu > image over raw devices" patch > http://lists.gnu.org/archive/html/qemu-devel/2008-12/msg01083.html > > I think we've fixed the security flaw (that was discovered but not > introduced by Shahar's patch). Doesn't the fmt= option to the block drivers achieve the same thing (except for not probing the backend formats)? Regards, Anthony Liguori > Also I'm attaching an examples file with some debugging messages. > > Thanks, > Uri. > > > ---------- > From: Uri Lublin > > The purpose of this prefix is to > 1. Provide a way to know the backing file format without probing > it (setting the format upon creation time). > 2. Enable using qcow2 format (and others) over host block devices. > (only if the user specifically asks for it). > > If no fmt:FMT: is provided we go back to probing. > > Based on a similar patch from Shahar Frank ("Qemu image over raw > devices"). > http://lists.gnu.org/archive/html/qemu-devel/2008-12/msg01083.html > > Also fixes a security flaw found by Daniel P. Berrange on the > above thread which summarizes: "Autoprobing: just say no." > > Examples: > > backing file format is qcow2 (even though it's on a host block device) > $ qemu-img create -b fmt:qcow2:/dev/loop0 -f qcow2 /tmp/uuu.qcow2 > > force backing file format to raw (no probing) > $ qemu-img create -f raw /tmp/image1.raw 10G > $ qemu-img create -b fmt:raw:/tmp/image1.raw -f qcow2 /tmp/image1.qcow2 > > Use together with other protocols, e.g. nbd > $ qemu-nbd -v -n --snapshot -t -k /tmp/uuu.socket > fmt:qcow2:/tmp/images/uuu.qcow2 & > $ qemu-img info nbd:unix:/tmp/uuu.socket > $ qemu-system-x86_64 -snapshot -hda nbd:unix:/tmp/uuu.socket > > Or fat > $ qemu-system-x86_64 -hda fmt:qcow2:/tmp/uuu.qcow2 -hdb > fat:floppy:/tmp/images > > Signed-off-by: Uri Lublin > --- > qemu/block.c | 72 > ++++++++++++++++++++++++++++++++++++++++++++++++--------- > 1 files changed, 60 insertions(+), 12 deletions(-) > > diff --git a/qemu/block.c b/qemu/block.c > index dc744dd..5b4c517 100644 > --- a/qemu/block.c > +++ b/qemu/block.c > @@ -72,7 +72,7 @@ int path_is_absolute(const char *path) > if (*path == '/' || *path == '\\') > return 1; > #endif > - p = strchr(path, ':'); > + p = strrchr(path, ':'); > if (p) > p++; > else > @@ -96,10 +96,23 @@ void path_combine(char *dest, int dest_size, > > if (dest_size <= 0) > return; > + > + /* copy "fmt:" prefix of filename if exists */ > + p = strrchr(filename, ':'); > + if (p) { > + len = p - filename + 1; > + if (dest_size <= len) > + return; > + strncpy(dest, filename, len); > + filename += len; > + dest += len; > + dest_size -= len; > + } > + > if (path_is_absolute(filename)) { > pstrcpy(dest, dest_size, filename); > } else { > - p = strchr(base_path, ':'); > + p = strrchr(base_path, ':'); > if (p) > p++; > else > @@ -227,30 +240,51 @@ static int is_windows_drive(const char *filename) > } > #endif > > +static const char *raw_filename(const char *filename) > +{ > + char *_filename; > + > + _filename = strrchr(filename, ':'); > + if (_filename) > + return _filename + 1; > + else > + return filename; > +} > + > static BlockDriver *find_protocol(const char *filename) > { > BlockDriver *drv1; > char protocol[128]; > - int len; > + int len, is_fmt = 0; > const char *p; > > + if (!strncmp(filename, "fmt:", 4)) { > + filename += 4; > + is_fmt = 1; > + } > + > #ifdef _WIN32 > if (is_windows_drive(filename) || > is_windows_drive_prefix(filename)) > - return &bdrv_raw; > + return NULL; > #endif > p = strchr(filename, ':'); > if (!p) > - return &bdrv_raw; > + return NULL; > len = p - filename; > if (len > sizeof(protocol) - 1) > len = sizeof(protocol) - 1; > memcpy(protocol, filename, len); > protocol[len] = '\0'; > for(drv1 = first_drv; drv1 != NULL; drv1 = drv1->next) { > - if (drv1->protocol_name && > - !strcmp(drv1->protocol_name, protocol)) > - return drv1; > + if (is_fmt) { > + if (!strcmp(drv1->format_name, protocol)) > + return drv1; > + } else { > + if (drv1->protocol_name && > + !strcmp(drv1->protocol_name, protocol)) > + return drv1; > + } > } > return NULL; > } > @@ -268,6 +302,14 @@ static BlockDriver *find_image_format(const char > *filename) > recognized as a host CDROM */ > if (strstart(filename, "/dev/cdrom", NULL)) > return &bdrv_host_device; > + > + drv = find_protocol(filename); > + if ((drv != NULL) && (drv->protocol_name == NULL)) > + return drv; > + > + if (drv == NULL) > + filename = raw_filename(filename); > + > #ifdef _WIN32 > if (is_windows_drive(filename)) > return &bdrv_host_device; > @@ -281,7 +323,6 @@ static BlockDriver *find_image_format(const char > *filename) > } > #endif > > - drv = find_protocol(filename); > /* no need to test disk image formats for vvfat */ > if (drv == &bdrv_vvfat) > return drv; > @@ -371,8 +412,11 @@ int bdrv_open2(BlockDriverState *bs, const char > *filename, int flags, > if (is_protocol) > snprintf(backing_filename, sizeof(backing_filename), > "%s", filename); > - else > - realpath(filename, backing_filename); > + else { > + const char *p; > + p = realpath(raw_filename(filename), backing_filename); > + path_combine(backing_filename, sizeof(backing_filename), > p, filename); > + } > > if (bdrv_create(&bdrv_qcow2, tmp_filename, > total_size, backing_filename, 0) < 0) { > @@ -386,7 +430,7 @@ int bdrv_open2(BlockDriverState *bs, const char > *filename, int flags, > if (flags & BDRV_O_FILE) { > drv = find_protocol(filename); > if (!drv) > - return -ENOENT; > + drv = &bdrv_raw; > } else { > if (!drv) { > drv = find_image_format(filename); > @@ -404,6 +448,10 @@ int bdrv_open2(BlockDriverState *bs, const char > *filename, int flags, > open_flags = BDRV_O_RDWR | (flags & BDRV_O_CACHE_MASK); > else > open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT); > + > + if (drv && !drv->protocol_name) > + filename = raw_filename(filename); > + > ret = drv->bdrv_open(bs, filename, open_flags); > if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) { > ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);