All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] qcow2: fix preallocation with metadata on bare block device
@ 2018-05-11 12:37 Ivan Ren
  2018-05-11 13:41 ` Eric Blake
  0 siblings, 1 reply; 3+ messages in thread
From: Ivan Ren @ 2018-05-11 12:37 UTC (permalink / raw)
  To: mreitz, kwolf; +Cc: qemu-block, qemu-devel

Create a qcow2 directly on bare block device with
"-o preallocation=metadata" option. When read this qcow2, it will
return pre-existing data on block device, and this may lead to
data leakage. This patch add QCOW_OFLAG_ZERO for all preallocated
l2 entry to avoid this problem.

Signed-off-by: Ivan Ren <ivanren@tencent.com>
---
Changes in v2:
- always pass QCOW_OFLAG_ZERO when preallocate metadta
---
 block/qcow2-cluster.c | 5 +++--
 block/qcow2.c         | 6 +++---
 block/qcow2.h         | 3 ++-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1aee726..b9e0ceb 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -919,7 +919,8 @@ fail:
     return ret;
 }
 
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
+int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
+                                uint64_t flags)
 {
     BDRVQcow2State *s = bs->opaque;
     int i, j = 0, l2_index, ret;
@@ -969,7 +970,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
         }
 
         l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
-                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
+                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED | flags);
      }
 
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 2f36e63..a7aeea9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2044,7 +2044,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
         while (l2meta != NULL) {
             QCowL2Meta *next;
 
-            ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
+            ret = qcow2_alloc_cluster_link_l2(bs, l2meta, 0);
             if (ret < 0) {
                 goto fail;
             }
@@ -2552,7 +2552,7 @@ static void coroutine_fn preallocate_co(void *opaque)
         while (meta) {
             QCowL2Meta *next = meta->next;
 
-            ret = qcow2_alloc_cluster_link_l2(bs, meta);
+            ret = qcow2_alloc_cluster_link_l2(bs, meta, QCOW_OFLAG_ZERO);
             if (ret < 0) {
                 qcow2_free_any_clusters(bs, meta->alloc_offset,
                                         meta->nb_clusters, QCOW2_DISCARD_NEVER);
@@ -3458,7 +3458,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
             };
             qemu_co_queue_init(&allocation.dependent_requests);
 
-            ret = qcow2_alloc_cluster_link_l2(bs, &allocation);
+            ret = qcow2_alloc_cluster_link_l2(bs, &allocation, 0);
             if (ret < 0) {
                 error_setg_errno(errp, -ret, "Failed to update L2 tables");
                 qcow2_free_clusters(bs, host_offset,
diff --git a/block/qcow2.h b/block/qcow2.h
index adf5c39..9a59602 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -617,7 +617,8 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
                                          uint64_t offset,
                                          int compressed_size);
 
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
+int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
+                                uint64_t flags);
 int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
                           uint64_t bytes, enum qcow2_discard_type type,
                           bool full_discard);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH v2] qcow2: fix preallocation with metadata on bare block device
  2018-05-11 12:37 [Qemu-devel] [PATCH v2] qcow2: fix preallocation with metadata on bare block device Ivan Ren
@ 2018-05-11 13:41 ` Eric Blake
  2018-05-11 14:40   ` Ivan Ren
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Blake @ 2018-05-11 13:41 UTC (permalink / raw)
  To: Ivan Ren, mreitz, kwolf; +Cc: qemu-devel, qemu-block

On 05/11/2018 07:37 AM, Ivan Ren wrote:
> Create a qcow2 directly on bare block device with
> "-o preallocation=metadata" option. When read this qcow2, it will
> return pre-existing data on block device, and this may lead to
> data leakage. This patch add QCOW_OFLAG_ZERO for all preallocated
> l2 entry to avoid this problem.

This is a semantic change; are we okay making it?

Does your code properly check for qcow2v2 files, which don't support 
QCOW_OFLAG_ZERO (only qcow2v3 supports it)?

> 
> Signed-off-by: Ivan Ren <ivanren@tencent.com>
> ---

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH v2] qcow2: fix preallocation with metadata on bare block device
  2018-05-11 13:41 ` Eric Blake
@ 2018-05-11 14:40   ` Ivan Ren
  0 siblings, 0 replies; 3+ messages in thread
From: Ivan Ren @ 2018-05-11 14:40 UTC (permalink / raw)
  To: eblake; +Cc: mreitz, kwolf, qemu-devel, qemu-block

>> Create a qcow2 directly on bare block device with
>> "-o preallocation=metadata" option. When read this qcow2, it will
>> return pre-existing data on block device, and this may lead to
>> data leakage. This patch add QCOW_OFLAG_ZERO for all preallocated
>> l2 entry to avoid this problem.
>
> This is a semantic change; are we okay making it?

> Does your code properly check for qcow2v2 files, which don't support
> QCOW_OFLAG_ZERO (only qcow2v3 supports it)?

Sorry for this mistake.
Current solution can only be used with s->qcow_version >= 3.
I'll fix it.

On Fri, May 11, 2018 at 9:41 PM Eric Blake <eblake@redhat.com> wrote:

> On 05/11/2018 07:37 AM, Ivan Ren wrote:
> > Create a qcow2 directly on bare block device with
> > "-o preallocation=metadata" option. When read this qcow2, it will
> > return pre-existing data on block device, and this may lead to
> > data leakage. This patch add QCOW_OFLAG_ZERO for all preallocated
> > l2 entry to avoid this problem.
>
> This is a semantic change; are we okay making it?
>
> Does your code properly check for qcow2v2 files, which don't support
> QCOW_OFLAG_ZERO (only qcow2v3 supports it)?
>
> >
> > Signed-off-by: Ivan Ren <ivanren@tencent.com>
> > ---
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-05-11 14:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-11 12:37 [Qemu-devel] [PATCH v2] qcow2: fix preallocation with metadata on bare block device Ivan Ren
2018-05-11 13:41 ` Eric Blake
2018-05-11 14:40   ` Ivan Ren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.