[Qemu-devel] [PATCH 1/2] Introducing qcow2 extensions + keep backing file format

[Uri Lublin]

Qcow2 extensions are build of magic (id) len (in bytes) and data.
They reside between the end of the header and the filename.

We can keep the backing file format in a such a qcow2 extension, to
1. Provide a way to know the backing file format without probing
   it (setting the format at creation time).
2. Enable using qcow2 format over host block devices.
   (only if the user specifically asks for it, by providing the format
   at creation time).

I've added bdrv_create2 and drv->bdrv_create2 (implemented only
by block-qcow2 currently) to pass the backing-format to create.

Based on a work done by Shahar Frank.

Also fixes a security flaw found by Daniel P. Berrange on [1]
which summarizes: "Autoprobing: just say no."

[1] http://lists.gnu.org/archive/html/qemu-devel/2008-12/msg01083.html

Signed-off-by: Uri Lublin <address@hidden>
---
 block-qcow2.c |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 block.c       |   29 +++++++++++++--
 block.h       |    4 ++
 block_int.h   |    6 +++
 4 files changed, 141 insertions(+), 6 deletions(-)

diff --git a/block-qcow2.c b/block-qcow2.c
index 465dcd6..222d423 100644
--- a/block-qcow2.c
+++ b/block-qcow2.c
@@ -45,6 +45,7 @@
 
 //#define DEBUG_ALLOC
 //#define DEBUG_ALLOC2
+//#define DEBUG_EXT
 
 #define QCOW_MAGIC (('Q' << 24) | ('F' << 16) | ('I' << 8) | 0xfb)
 #define QCOW_VERSION 2
@@ -77,6 +78,14 @@ typedef struct QCowHeader {
     uint64_t snapshots_offset;
 } QCowHeader;
 
+
+typedef struct {
+    uint32_t magic;
+    uint32_t len;
+} QCowExtension;
+#define  QCOW_EXT_MAGIC_END 0
+#define  QCOW_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
+
 typedef struct __attribute__((packed)) QCowSnapshotHeader {
     /* header is 8 byte aligned */
     uint64_t l1_table_offset;
@@ -189,6 +198,66 @@ static int qcow_probe(const uint8_t *buf, int buf_size, 
const char *filename)
         return 0;
 }
 
+
+/* read qcow2 extension and fill bs
+ * start reading from start_offset
+ * finish reading upon magic of value 0 or when end_offset reached
+ * unknown magic is skipped (future extension this version knows nothing about)
+ * return 0 upon success, non-0 otherwise
+ */
+static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset,
+                                uint64_t end_offset)
+{
+    BDRVQcowState *s = bs->opaque;
+    QCowExtension ext;
+    uint64_t offset;
+
+#ifdef DEBUG_EXT
+    printf("qcow_read_extensions: start=%ld end=%ld\n", start_offset, 
end_offset);
+#endif
+    offset = start_offset;
+    while (offset < end_offset) {
+
+#ifdef DEBUG_EXT
+        /* Sanity check */
+        if (offset >= s->cluster_size)
+            printf("qcow_handle_extension: suspicious offset %lu\n", offset);
+
+        printf("attemting to read extended header in offset %lu\n", offset);
+#endif
+
+        if (bdrv_pread(s->hd, offset, &ext, sizeof(ext)) != sizeof(ext)) {
+            fprintf(stderr, "qcow_handle_extension: ERROR: pread fail from 
offset %lu\n",
+                    offset);
+            return 1;
+        }
+        be32_to_cpus(&ext.magic);
+        be32_to_cpus(&ext.len);
+        offset += sizeof(ext);
+        switch (ext.magic) {
+        case QCOW_EXT_MAGIC_END:
+            break;
+        case QCOW_EXT_MAGIC_BACKING_FORMAT:
+            if (ext.len >= sizeof(bs->backing_format)) {
+                fprintf(stderr, "ERROR: ext_backing_format: len=%u too large"
+                        " (>=%lu)\n",
+                        ext.len, sizeof(bs->backing_format));
+                return 2;
+            }
+            if (bdrv_pread(s->hd, offset , bs->backing_format,
+                           ext.len) != ext.len)
+                return 3;
+            bs->backing_format[ext.len] = '\0';
+#ifdef DEBUG_EXT
+            printf("Qcow2: Got format extension %s\n", bs->backing_format);
+#endif
+            offset += ((ext.len + 7) & ~7);
+        }
+    }
+
+    return 0;
+}
+
 static int qcow_open(BlockDriverState *bs, const char *filename, int flags)
 {
     BDRVQcowState *s = bs->opaque;
@@ -278,6 +347,10 @@ static int qcow_open(BlockDriverState *bs, const char 
*filename, int flags)
 
     scan_refcount(bs, &s->highest_alloc, &s->nc_free);
 
+    /* read qcow2 extensions */
+    if (qcow_read_extensions(bs, sizeof(header), header.backing_file_offset))
+        goto fail;
+
     /* read the backing file name */
     if (header.backing_file_offset != 0) {
         len = header.backing_file_size;
@@ -1462,13 +1535,17 @@ static void create_refcount_update(QCowCreateState *s,
     }
 }
 
-static int qcow_create(const char *filename, int64_t total_size,
-                      const char *backing_file, int flags)
+static int qcow_create2(const char *filename, int64_t total_size,
+                        const char *backing_file, const char *backing_format,
+                        int flags)
 {
     int fd, header_size, backing_filename_len, l1_size, i, shift, l2_bits;
+    int backing_format_len = 0;
     QCowHeader header;
     uint64_t tmp, offset;
     QCowCreateState s1, *s = &s1;
+    QCowExtension ext = {0, 0};
+
 
     memset(s, 0, sizeof(*s));
 
@@ -1482,6 +1559,12 @@ static int qcow_create(const char *filename, int64_t 
total_size,
     header_size = sizeof(header);
     backing_filename_len = 0;
     if (backing_file) {
+        if (backing_format) {
+            ext.magic = QCOW_EXT_MAGIC_BACKING_FORMAT;
+            backing_format_len = strlen(backing_format);
+            ext.len = (backing_format_len + 7) & ~7;
+            header_size += ((sizeof(ext) + ext.len + 7) & ~7);
+        }
         header.backing_file_offset = cpu_to_be64(header_size);
         backing_filename_len = strlen(backing_file);
         header.backing_file_size = cpu_to_be32(backing_filename_len);
@@ -1526,6 +1609,19 @@ static int qcow_create(const char *filename, int64_t 
total_size,
     /* write all the data */
     write(fd, &header, sizeof(header));
     if (backing_file) {
+        if (backing_format_len) {
+            char zero[16];
+            int d = ext.len - backing_format_len;
+
+            memset(zero, 0, sizeof(zero));
+            cpu_to_be32s(&ext.magic);
+            cpu_to_be32s(&ext.len);
+            write(fd, &ext, sizeof(ext));
+            write(fd, backing_format, backing_format_len);
+            if (d>0) {
+                write(fd, zero, d);
+            }
+        }
         write(fd, backing_file, backing_filename_len);
     }
     lseek(fd, s->l1_table_offset, SEEK_SET);
@@ -1545,6 +1641,12 @@ static int qcow_create(const char *filename, int64_t 
total_size,
     return 0;
 }
 
+static int qcow_create(const char *filename, int64_t total_size,
+                       const char *backing_file, int flags)
+{
+    return qcow_create2(filename, total_size, backing_file, NULL, flags);
+}
+
 static int qcow_make_empty(BlockDriverState *bs)
 {
 #if 0
@@ -2670,4 +2772,6 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_snapshot_delete = qcow_snapshot_delete,
     .bdrv_snapshot_list = qcow_snapshot_list,
     .bdrv_get_info = qcow_get_info,
+
+    .bdrv_create2 = qcow_create2,
 };
diff --git a/block.c b/block.c
index 4f4bf7c..0939cad 100644
--- a/block.c
+++ b/block.c
@@ -172,6 +172,20 @@ BlockDriver *bdrv_find_format(const char *format_name)
     return NULL;
 }
 
+int bdrv_create2(BlockDriver *drv,
+                const char *filename, int64_t size_in_sectors,
+                const char *backing_file, const char *backing_format,
+                int flags)
+{
+    if (drv->bdrv_create2)
+        return drv->bdrv_create2(filename, size_in_sectors, backing_file,
+                                 backing_format, flags);
+    if (drv->bdrv_create)
+        return drv->bdrv_create(filename, size_in_sectors, backing_file,
+                                flags);
+    return -ENOTSUP;
+}
+
 int bdrv_create(BlockDriver *drv,
                 const char *filename, int64_t size_in_sectors,
                 const char *backing_file, int flags)
@@ -351,7 +365,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
         if (!bs1) {
             return -ENOMEM;
         }
-        if (bdrv_open(bs1, filename, 0) < 0) {
+        if (bdrv_open2(bs1, filename, 0, drv) < 0) {
             bdrv_delete(bs1);
             return -1;
         }
@@ -371,11 +385,14 @@ int bdrv_open2(BlockDriverState *bs, const char 
*filename, int flags,
         else
             realpath(filename, backing_filename);
 
-        if (bdrv_create(&bdrv_qcow2, tmp_filename,
-                        total_size, backing_filename, 0) < 0) {
+        if (bdrv_create2(&bdrv_qcow2, tmp_filename,
+                         total_size, backing_filename,
+                         (drv ? drv->format_name : NULL),
+                         0) < 0) {
             return -1;
         }
         filename = tmp_filename;
+        drv = &bdrv_qcow2;
         bs->is_temporary = 1;
     }
 
@@ -420,6 +437,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
 #endif
     if (bs->backing_file[0] != '\0') {
         /* if there is a backing file, use it */
+        BlockDriver *back_drv = NULL;
         bs->backing_hd = bdrv_new("");
         if (!bs->backing_hd) {
         fail:
@@ -428,7 +446,10 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
         }
         path_combine(backing_filename, sizeof(backing_filename),
                      filename, bs->backing_file);
-        if (bdrv_open(bs->backing_hd, backing_filename, open_flags) < 0)
+        if (bs->backing_format)
+            back_drv = bdrv_find_format(bs->backing_format);
+        if (bdrv_open2(bs->backing_hd, backing_filename, open_flags,
+                       back_drv) < 0)
             goto fail;
     }
 
diff --git a/block.h b/block.h
index e1927dd..e19fd8f 100644
--- a/block.h
+++ b/block.h
@@ -64,6 +64,10 @@ BlockDriver *bdrv_find_format(const char *format_name);
 int bdrv_create(BlockDriver *drv,
                 const char *filename, int64_t size_in_sectors,
                 const char *backing_file, int flags);
+int bdrv_create2(BlockDriver *drv,
+                 const char *filename, int64_t size_in_sectors,
+                 const char *backing_file, const char *backing_format,
+                 int flags);
 BlockDriverState *bdrv_new(const char *device_name);
 void bdrv_delete(BlockDriverState *bs);
 int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
diff --git a/block_int.h b/block_int.h
index 781789c..8c373ff 100644
--- a/block_int.h
+++ b/block_int.h
@@ -85,6 +85,11 @@ struct BlockDriver {
     /* to control generic scsi devices */
     int (*bdrv_ioctl)(BlockDriverState *bs, unsigned long int req, void *buf);
 
+    /* new create with backing file format */
+    int (*bdrv_create2)(const char *filename, int64_t total_sectors,
+                        const char *backing_file, const char *backing_format,
+                        int flags);
+
     BlockDriverAIOCB *free_aiocb;
     struct BlockDriver *next;
 };
@@ -107,6 +112,7 @@ struct BlockDriverState {
     char filename[1024];
     char backing_file[1024]; /* if non zero, the image is a diff of
                                 this file image */
+    char backing_format[16]; /* if non-zero and backing_file exists */
     int is_temporary;
     int media_changed;
 
-- 
1.6.0.6