[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PULL 23/71] block: fix bdrv_check_perm for non-tree subgra
From: |
Kevin Wolf |
Subject: |
[Qemu-block] [PULL 23/71] block: fix bdrv_check_perm for non-tree subgraph |
Date: |
Mon, 25 Feb 2019 16:20:05 +0100 |
From: Vladimir Sementsov-Ogievskiy <address@hidden>
bdrv_check_perm in it's recursion checks each node in context of new
permissions for one parent, because of nature of DFS. It works well,
while children subgraph of top-most updated node is a tree, i.e. it
doesn't have any kind of loops. But if we have a loop (not oriented,
of course), i.e. we have two different ways from top-node to some
child-node, then bdrv_check_perm will do wrong thing:
top
| \
| |
v v
A B
| |
v v
node
It will once check new permissions of node in context of new A
permissions and old B permissions and once visa-versa. It's a wrong way
and may lead to corruption of permission system. We may start with
no-permissions and all-shared for both A->node and B->node relations
and finish up with non shared write permission for both ways.
The following commit will add a test, which shows this bug.
To fix this situation, let's really set BdrvChild permissions during
bdrv_check_perm procedure. And we are happy here, as check-perm is
already written in transaction manner, so we just need to restore
backed-up permissions in _abort.
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
include/block/block_int.h | 5 +++++
block.c | 27 ++++++++++++++++++++++++++-
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0075bafd10..8437df85a2 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -662,6 +662,11 @@ struct BdrvChild {
*/
uint64_t shared_perm;
+ /* backup of permissions during permission update procedure */
+ bool has_backup_perm;
+ uint64_t backup_perm;
+ uint64_t backup_shared_perm;
+
QLIST_ENTRY(BdrvChild) next;
QLIST_ENTRY(BdrvChild) next_parent;
};
diff --git a/block.c b/block.c
index bb4bf1237c..16d59e0b32 100644
--- a/block.c
+++ b/block.c
@@ -1954,13 +1954,32 @@ static int bdrv_child_check_perm(BdrvChild *c,
BlockReopenQueue *q,
ret = bdrv_check_update_perm(c->bs, q, perm, shared, ignore_children,
errp);
g_slist_free(ignore_children);
- return ret;
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (!c->has_backup_perm) {
+ c->has_backup_perm = true;
+ c->backup_perm = c->perm;
+ c->backup_shared_perm = c->shared_perm;
+ }
+ /*
+ * Note: it's OK if c->has_backup_perm was already set, as we can find the
+ * same child twice during check_perm procedure
+ */
+
+ c->perm = perm;
+ c->shared_perm = shared;
+
+ return 0;
}
static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared)
{
uint64_t cumulative_perms, cumulative_shared_perms;
+ c->has_backup_perm = false;
+
c->perm = perm;
c->shared_perm = shared;
@@ -1971,6 +1990,12 @@ static void bdrv_child_set_perm(BdrvChild *c, uint64_t
perm, uint64_t shared)
static void bdrv_child_abort_perm_update(BdrvChild *c)
{
+ if (c->has_backup_perm) {
+ c->perm = c->backup_perm;
+ c->shared_perm = c->backup_shared_perm;
+ c->has_backup_perm = false;
+ }
+
bdrv_abort_perm_update(c->bs);
}
--
2.20.1
- [Qemu-block] [PULL 13/71] io: Remove redundant read/write_coroutine assignments, (continued)
- [Qemu-block] [PULL 13/71] io: Remove redundant read/write_coroutine assignments, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 14/71] nbd: Move nbd_read_eof() to nbd/client.c, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 18/71] block: Fix AioContext switch for drained node, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 20/71] block: Use normal drain for bdrv_set_aio_context(), Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 16/71] nbd: Increase bs->in_flight during AioContext switch, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 17/71] block: Don't poll in bdrv_set_aio_context(), Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 15/71] nbd: Use low-level QIOChannel API in nbd_read_eof(), Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 19/71] test-bdrv-drain: AioContext switch in drained section, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 21/71] aio-posix: Assert that aio_poll() is always called in home thread, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 22/71] block: improve should_update_child, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 23/71] block: fix bdrv_check_perm for non-tree subgraph,
Kevin Wolf <=
- [Qemu-block] [PULL 24/71] tests: add test-bdrv-graph-mod, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 25/71] qcow2: Assert that L2 table offsets fit in the L1 table, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 32/71] iotests.py: Add filter_imgfmt(), Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 29/71] block: Skip implicit nodes for filename info, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 36/71] block: bdrv_get_full_backing_filename_from_...'s ret. val., Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 38/71] block: Add bdrv_make_absolute_filename(), Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 33/71] iotests.py: Add node_info(), Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 35/71] block: Make path_combine() return the path, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 28/71] block: Use children list in bdrv_refresh_filename, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 37/71] block: bdrv_get_full_backing_filename's ret. val., Kevin Wolf, 2019/02/25