[Qemu-devel] [PATCH 15/17] qcow2-dirty-bitmaps: handle store reqursion

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

If persistent dirty bitmap bm tracks bs->file and stored in bs, then
saving this bitmap to the image will change (make bits dirty) the
bitmap bm. This is strange behaviour and should be forbidden.

RFC:
Should we check cases like
bs_for == bs_file->file->file, or bs_for->file == bs_file, or
bs_for->file == bs_file->file->file, etc?

The most common check would be

if bs_for == bs_file - it's ok
else
  if bs_for[->file...] == bs_file[->file...] - it's bad
  else - it's ok

so, there two 'ok' cases: bs_for and bs_file are the same or they are
absolutely unrelated.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c                    | 12 ++++++++++++
 block/qcow2-dirty-bitmap.c | 12 ++++++++++++
 include/block/block.h      |  1 +
 3 files changed, 25 insertions(+)

diff --git a/block.c b/block.c
index 9148977..df95bf9 100644
--- a/block.c
+++ b/block.c
@@ -3574,6 +3574,18 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
     }
 }
 
+bool bdrv_has_dirty_bitmap(BlockDriverState *bs, const BdrvDirtyBitmap *bitmap)
+{
+    BdrvDirtyBitmap *bm, *next;
+    QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
+        if (bm == bitmap) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 void bdrv_dirty_bitmap_set_file(BdrvDirtyBitmap *bitmap, BlockDriverState *file)
 {
     assert(bitmap->file == NULL);
diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index 3d3c624..d38f15f 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -346,6 +346,13 @@ BdrvDirtyBitmap * qcow2_dirty_bitmap_load(BlockDriverState *bs_for,
     uint64_t size = bdrv_nb_sectors(bs_for);
     BdrvDirtyBitmap *bitmap = NULL;
 
+    /* reqursive storing is not allowed */
+    if (bs_for == bs_file->file) {
+        error_setg(errp, "Bitmap store recursion detected for bitmap '%s'",
+                   name);
+        return NULL;
+    }
+
     bm = find_dirty_bitmap_by_name(bs_file, name);
     if (bm == NULL) {
         error_setg(errp, "Could not find bitmap '%s' in the node '%s'", name,
@@ -770,6 +777,11 @@ int qcow2_dirty_bitmap_store(BlockDriverState *bs, const BdrvDirtyBitmap *bitmap
     uint64_t size = bdrv_dirty_bitmap_size(bitmap);
     int granularity = bdrv_dirty_bitmap_granularity(bitmap);
 
+    /* reqursive storing is not allowed */
+    if (bdrv_has_dirty_bitmap(bs->file, bitmap)) {
+        return -EINVAL;
+    }
+
     /* find/create dirty bitmap */
     bm = find_dirty_bitmap_by_name(bs, name);
     if (bm == NULL) {
diff --git a/include/block/block.h b/include/block/block.h
index f587a03..67a7f0c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -491,6 +491,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
                                         const char *name);
 void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+bool bdrv_has_dirty_bitmap(BlockDriverState *bs, const BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_bitmap_set_file(BdrvDirtyBitmap *bitmap,
                                 BlockDriverState *file);
 void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
-- 
2.1.4