block: Fail gracefully when blockdev-snapshot creates loops
Using blockdev-snapshot to append a node as an overlay to itself, or to
any of its parents, causes crashes. Catch the condition and return an
error for these cases instead.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1824363
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20211018134714.48438-1-kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
diff --git a/block.c b/block.c
index 45f653a..580cb77 100644
--- a/block.c
+++ b/block.c
@@ -84,6 +84,9 @@
BdrvChildRole child_role,
Error **errp);
+static bool bdrv_recurse_has_child(BlockDriverState *bs,
+ BlockDriverState *child);
+
static void bdrv_replace_child_noperm(BdrvChild *child,
BlockDriverState *new_bs);
static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
@@ -2673,6 +2676,7 @@
int drain_saldo;
assert(!child->frozen);
+ assert(old_bs != new_bs);
if (old_bs && new_bs) {
assert(bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs));
@@ -2892,6 +2896,12 @@
assert(parent_bs->drv);
+ if (bdrv_recurse_has_child(child_bs, parent_bs)) {
+ error_setg(errp, "Making '%s' a %s child of '%s' would create a cycle",
+ child_bs->node_name, child_name, parent_bs->node_name);
+ return -EINVAL;
+ }
+
bdrv_get_cumulative_perm(parent_bs, &perm, &shared_perm);
bdrv_child_perm(parent_bs, child_bs, NULL, child_role, NULL,
perm, shared_perm, &perm, &shared_perm);
diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index d557522..de74262 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -103,11 +103,18 @@
}
# ${1}: unique identifier for the snapshot filename
-add_snapshot_image()
+create_snapshot_image()
{
base_image="${TEST_DIR}/$((${1}-1))-${snapshot_virt0}"
snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}"
TEST_IMG=$snapshot_file _make_test_img -u -b "${base_image}" -F $IMGFMT "$size"
+}
+
+# ${1}: unique identifier for the snapshot filename
+add_snapshot_image()
+{
+ snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}"
+ create_snapshot_image "$1"
do_blockdev_add "$1" "'backing': null, " "${snapshot_file}"
}
@@ -231,6 +238,28 @@
blockdev_snapshot ${SNAPSHOTS} error
echo
+echo === Invalid command - creating loops ===
+echo
+
+SNAPSHOTS=$((${SNAPSHOTS}+1))
+add_snapshot_image ${SNAPSHOTS}
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+ 'arguments': { 'node':'snap_${SNAPSHOTS}',
+ 'overlay':'snap_${SNAPSHOTS}' }
+ }" "error"
+
+SNAPSHOTS=$((${SNAPSHOTS}+1))
+create_snapshot_image ${SNAPSHOTS}
+do_blockdev_add ${SNAPSHOTS} "'backing': 'snap_$((${SNAPSHOTS}-1))', " \
+ "${TEST_DIR}/${SNAPSHOTS}-${snapshot_virt0}"
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+ 'arguments': { 'node':'snap_${SNAPSHOTS}',
+ 'overlay':'snap_$((${SNAPSHOTS}-1))' }
+ }" "error"
+
+echo
echo === Invalid command - The node does not exist ===
echo
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index 1d4c565..b543b99 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -217,15 +217,42 @@
'overlay':'snap_13' } }
{"error": {"class": "GenericError", "desc": "The overlay already has a backing image"}}
+=== Invalid command - creating loops ===
+
+Formatting 'TEST_DIR/14-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/13-snapshot-v0.IMGFMT backing_fmt=IMGFMT
+{ 'execute': 'blockdev-add', 'arguments':
+ { 'driver': 'IMGFMT', 'node-name': 'snap_14', 'backing': null,
+ 'file':
+ { 'driver': 'file', 'filename': 'TEST_DIR/14-snapshot-v0.IMGFMT',
+ 'node-name': 'file_14' } } }
+{"return": {}}
+{ 'execute': 'blockdev-snapshot',
+ 'arguments': { 'node':'snap_14',
+ 'overlay':'snap_14' }
+ }
+{"error": {"class": "GenericError", "desc": "Making 'snap_14' a backing child of 'snap_14' would create a cycle"}}
+Formatting 'TEST_DIR/15-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/14-snapshot-v0.IMGFMT backing_fmt=IMGFMT
+{ 'execute': 'blockdev-add', 'arguments':
+ { 'driver': 'IMGFMT', 'node-name': 'snap_15', 'backing': 'snap_14',
+ 'file':
+ { 'driver': 'file', 'filename': 'TEST_DIR/15-snapshot-v0.IMGFMT',
+ 'node-name': 'file_15' } } }
+{"return": {}}
+{ 'execute': 'blockdev-snapshot',
+ 'arguments': { 'node':'snap_15',
+ 'overlay':'snap_14' }
+ }
+{"error": {"class": "GenericError", "desc": "Making 'snap_15' a backing child of 'snap_14' would create a cycle"}}
+
=== Invalid command - The node does not exist ===
{ 'execute': 'blockdev-snapshot',
'arguments': { 'node': 'virtio0',
- 'overlay':'snap_14' } }
-{"error": {"class": "GenericError", "desc": "Cannot find device='snap_14' nor node-name='snap_14'"}}
+ 'overlay':'snap_16' } }
+{"error": {"class": "GenericError", "desc": "Cannot find device='snap_16' nor node-name='snap_16'"}}
{ 'execute': 'blockdev-snapshot',
'arguments': { 'node':'nodevice',
- 'overlay':'snap_13' }
+ 'overlay':'snap_15' }
}
{"error": {"class": "GenericError", "desc": "Cannot find device='nodevice' nor node-name='nodevice'"}}
*** done
