diff options
author | Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 2019-02-23 22:20:40 +0300 |
---|---|---|
committer | Kevin Wolf <kwolf@redhat.com> | 2019-02-25 15:03:19 +0100 |
commit | f962e96150e9c6a41e26caeaf93a65ec5b755607 (patch) | |
tree | 0043e65f73d0ada51a7230afeb0ad111a9dd3dc2 /block.c | |
parent | 2f30b7c377fa9a7dfbaf6eed56a07be7953e509e (diff) | |
download | qemu-f962e96150e9c6a41e26caeaf93a65ec5b755607.zip qemu-f962e96150e9c6a41e26caeaf93a65ec5b755607.tar.gz qemu-f962e96150e9c6a41e26caeaf93a65ec5b755607.tar.bz2 |
block: fix bdrv_check_perm for non-tree subgraph
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 <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Diffstat (limited to 'block.c')
-rw-r--r-- | block.c | 27 |
1 files changed, 26 insertions, 1 deletions
@@ -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); } |