| Message ID | 1385462881-12290-1-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2013年11月26日 18:48, Kevin Wolf wrote: > Strictly speaking, this is only required for has_zero_init() == false, > but it's easy enough to just do a cluster-aligned write that is padded > with zeros after the header. > > This fixes that after 'qemu-img create' header extensions are attempted > to be parsed that are really just random leftover data. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block/qcow2.c | 37 +++++++++++++++++++++---------------- > 1 file changed, 21 insertions(+), 16 deletions(-) > > diff --git a/block/qcow2.c b/block/qcow2.c > index 6e5d98d..7c18587 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1471,7 +1471,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, > * size for any qcow2 image. > */ > BlockDriverState* bs; > - QCowHeader header; > + QCowHeader *header; > uint8_t* refcount_table; > Error *local_err = NULL; > int ret; > @@ -1489,30 +1489,35 @@ static int qcow2_create2(const char *filename, int64_t total_size, > } > > /* Write the header */ > - memset(&header, 0, sizeof(header)); > - header.magic = cpu_to_be32(QCOW_MAGIC); > - header.version = cpu_to_be32(version); > - header.cluster_bits = cpu_to_be32(cluster_bits); > - header.size = cpu_to_be64(0); > - header.l1_table_offset = cpu_to_be64(0); > - header.l1_size = cpu_to_be32(0); > - header.refcount_table_offset = cpu_to_be64(cluster_size); > - header.refcount_table_clusters = cpu_to_be32(1); > - header.refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT); > - header.header_length = cpu_to_be32(sizeof(header)); > + QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header)); > + header = g_malloc(cluster_size); > + memset(header, 0, cluster_size); Could just be: header = g_malloc0(cluster_size); But either way, Reviewed-by: Fam Zheng <famz@redhat.com> > + *header = (QCowHeader) { > + .magic = cpu_to_be32(QCOW_MAGIC), > + .version = cpu_to_be32(version), > + .cluster_bits = cpu_to_be32(cluster_bits), > + .size = cpu_to_be64(0), > + .l1_table_offset = cpu_to_be64(0), > + .l1_size = cpu_to_be32(0), > + .refcount_table_offset = cpu_to_be64(cluster_size), > + .refcount_table_clusters = cpu_to_be32(1), > + .refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT), > + .header_length = cpu_to_be32(sizeof(*header)), > + }; > > if (flags & BLOCK_FLAG_ENCRYPT) { > - header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES); > + header->crypt_method = cpu_to_be32(QCOW_CRYPT_AES); > } else { > - header.crypt_method = cpu_to_be32(QCOW_CRYPT_NONE); > + header->crypt_method = cpu_to_be32(QCOW_CRYPT_NONE); > } > > if (flags & BLOCK_FLAG_LAZY_REFCOUNTS) { > - header.compatible_features |= > + header->compatible_features |= > cpu_to_be64(QCOW2_COMPAT_LAZY_REFCOUNTS); > } > > - ret = bdrv_pwrite(bs, 0, &header, sizeof(header)); > + ret = bdrv_pwrite(bs, 0, header, cluster_size); > + g_free(header); > if (ret < 0) { > error_setg_errno(errp, -ret, "Could not write qcow2 header"); > goto out; >
Il 26/11/2013 11:48, Kevin Wolf ha scritto: > Strictly speaking, this is only required for has_zero_init() == false, > but it's easy enough to just do a cluster-aligned write that is padded > with zeros after the header. > > This fixes that after 'qemu-img create' header extensions are attempted > to be parsed that are really just random leftover data. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block/qcow2.c | 37 +++++++++++++++++++++---------------- > 1 file changed, 21 insertions(+), 16 deletions(-) > > diff --git a/block/qcow2.c b/block/qcow2.c > index 6e5d98d..7c18587 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1471,7 +1471,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, > * size for any qcow2 image. > */ > BlockDriverState* bs; > - QCowHeader header; > + QCowHeader *header; > uint8_t* refcount_table; > Error *local_err = NULL; > int ret; > @@ -1489,30 +1489,35 @@ static int qcow2_create2(const char *filename, int64_t total_size, > } > > /* Write the header */ > - memset(&header, 0, sizeof(header)); > - header.magic = cpu_to_be32(QCOW_MAGIC); > - header.version = cpu_to_be32(version); > - header.cluster_bits = cpu_to_be32(cluster_bits); > - header.size = cpu_to_be64(0); > - header.l1_table_offset = cpu_to_be64(0); > - header.l1_size = cpu_to_be32(0); > - header.refcount_table_offset = cpu_to_be64(cluster_size); > - header.refcount_table_clusters = cpu_to_be32(1); > - header.refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT); > - header.header_length = cpu_to_be32(sizeof(header)); > + QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header)); > + header = g_malloc(cluster_size); > + memset(header, 0, cluster_size); > + *header = (QCowHeader) { > + .magic = cpu_to_be32(QCOW_MAGIC), > + .version = cpu_to_be32(version), > + .cluster_bits = cpu_to_be32(cluster_bits), > + .size = cpu_to_be64(0), > + .l1_table_offset = cpu_to_be64(0), > + .l1_size = cpu_to_be32(0), > + .refcount_table_offset = cpu_to_be64(cluster_size), > + .refcount_table_clusters = cpu_to_be32(1), > + .refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT), > + .header_length = cpu_to_be32(sizeof(*header)), > + }; > > if (flags & BLOCK_FLAG_ENCRYPT) { > - header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES); > + header->crypt_method = cpu_to_be32(QCOW_CRYPT_AES); > } else { > - header.crypt_method = cpu_to_be32(QCOW_CRYPT_NONE); > + header->crypt_method = cpu_to_be32(QCOW_CRYPT_NONE); > } > > if (flags & BLOCK_FLAG_LAZY_REFCOUNTS) { > - header.compatible_features |= > + header->compatible_features |= > cpu_to_be64(QCOW2_COMPAT_LAZY_REFCOUNTS); > } > > - ret = bdrv_pwrite(bs, 0, &header, sizeof(header)); > + ret = bdrv_pwrite(bs, 0, header, cluster_size); > + g_free(header); > if (ret < 0) { > error_setg_errno(errp, -ret, "Could not write qcow2 header"); > goto out; >
Am 26.11.2013 um 13:18 hat Fam Zheng geschrieben: > On 2013年11月26日 18:48, Kevin Wolf wrote: > >Strictly speaking, this is only required for has_zero_init() == false, > >but it's easy enough to just do a cluster-aligned write that is padded > >with zeros after the header. > > > >This fixes that after 'qemu-img create' header extensions are attempted > >to be parsed that are really just random leftover data. > > > >Cc: qemu-stable@nongnu.org > >Signed-off-by: Kevin Wolf <kwolf@redhat.com> > >--- > > block/qcow2.c | 37 +++++++++++++++++++++---------------- > > 1 file changed, 21 insertions(+), 16 deletions(-) > > > >diff --git a/block/qcow2.c b/block/qcow2.c > >index 6e5d98d..7c18587 100644 > >--- a/block/qcow2.c > >+++ b/block/qcow2.c > >@@ -1471,7 +1471,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, > > * size for any qcow2 image. > > */ > > BlockDriverState* bs; > >- QCowHeader header; > >+ QCowHeader *header; > > uint8_t* refcount_table; > > Error *local_err = NULL; > > int ret; > >@@ -1489,30 +1489,35 @@ static int qcow2_create2(const char *filename, int64_t total_size, > > } > > > > /* Write the header */ > >- memset(&header, 0, sizeof(header)); > >- header.magic = cpu_to_be32(QCOW_MAGIC); > >- header.version = cpu_to_be32(version); > >- header.cluster_bits = cpu_to_be32(cluster_bits); > >- header.size = cpu_to_be64(0); > >- header.l1_table_offset = cpu_to_be64(0); > >- header.l1_size = cpu_to_be32(0); > >- header.refcount_table_offset = cpu_to_be64(cluster_size); > >- header.refcount_table_clusters = cpu_to_be32(1); > >- header.refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT); > >- header.header_length = cpu_to_be32(sizeof(header)); > >+ QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header)); > >+ header = g_malloc(cluster_size); > >+ memset(header, 0, cluster_size); > > Could just be: > > header = g_malloc0(cluster_size); > > But either way, Yes, that's better. I'll change it. > Reviewed-by: Fam Zheng <famz@redhat.com> Thanks. Kevin
diff --git a/block/qcow2.c b/block/qcow2.c index 6e5d98d..7c18587 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1471,7 +1471,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, * size for any qcow2 image. */ BlockDriverState* bs; - QCowHeader header; + QCowHeader *header; uint8_t* refcount_table; Error *local_err = NULL; int ret; @@ -1489,30 +1489,35 @@ static int qcow2_create2(const char *filename, int64_t total_size, } /* Write the header */ - memset(&header, 0, sizeof(header)); - header.magic = cpu_to_be32(QCOW_MAGIC); - header.version = cpu_to_be32(version); - header.cluster_bits = cpu_to_be32(cluster_bits); - header.size = cpu_to_be64(0); - header.l1_table_offset = cpu_to_be64(0); - header.l1_size = cpu_to_be32(0); - header.refcount_table_offset = cpu_to_be64(cluster_size); - header.refcount_table_clusters = cpu_to_be32(1); - header.refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT); - header.header_length = cpu_to_be32(sizeof(header)); + QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header)); + header = g_malloc(cluster_size); + memset(header, 0, cluster_size); + *header = (QCowHeader) { + .magic = cpu_to_be32(QCOW_MAGIC), + .version = cpu_to_be32(version), + .cluster_bits = cpu_to_be32(cluster_bits), + .size = cpu_to_be64(0), + .l1_table_offset = cpu_to_be64(0), + .l1_size = cpu_to_be32(0), + .refcount_table_offset = cpu_to_be64(cluster_size), + .refcount_table_clusters = cpu_to_be32(1), + .refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT), + .header_length = cpu_to_be32(sizeof(*header)), + }; if (flags & BLOCK_FLAG_ENCRYPT) { - header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES); + header->crypt_method = cpu_to_be32(QCOW_CRYPT_AES); } else { - header.crypt_method = cpu_to_be32(QCOW_CRYPT_NONE); + header->crypt_method = cpu_to_be32(QCOW_CRYPT_NONE); } if (flags & BLOCK_FLAG_LAZY_REFCOUNTS) { - header.compatible_features |= + header->compatible_features |= cpu_to_be64(QCOW2_COMPAT_LAZY_REFCOUNTS); } - ret = bdrv_pwrite(bs, 0, &header, sizeof(header)); + ret = bdrv_pwrite(bs, 0, header, cluster_size); + g_free(header); if (ret < 0) { error_setg_errno(errp, -ret, "Could not write qcow2 header"); goto out;
Strictly speaking, this is only required for has_zero_init() == false, but it's easy enough to just do a cluster-aligned write that is padded with zeros after the header. This fixes that after 'qemu-img create' header extensions are attempted to be parsed that are really just random leftover data. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow2.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-)