From nobody Thu Dec 18 13:18:45 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1551110171586657.5948316078916; Mon, 25 Feb 2019 07:56:11 -0800 (PST) Received: from localhost ([127.0.0.1]:39557 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gyIbt-0003Kd-Ey for importer@patchew.org; Mon, 25 Feb 2019 10:56:05 -0500 Received: from eggs.gnu.org ([209.51.188.92]:37479) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gyI54-0000Wf-Dg for qemu-devel@nongnu.org; Mon, 25 Feb 2019 10:22:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gyI52-00018q-9K for qemu-devel@nongnu.org; Mon, 25 Feb 2019 10:22:10 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46514) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gyI4h-0000rs-2q; Mon, 25 Feb 2019 10:21:47 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AD019821DE; Mon, 25 Feb 2019 15:21:43 +0000 (UTC) Received: from linux.fritz.box.com (ovpn-117-243.ams2.redhat.com [10.36.117.243]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9D8395C221; Mon, 25 Feb 2019 15:21:42 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Mon, 25 Feb 2019 16:20:05 +0100 Message-Id: <20190225152053.15976-24-kwolf@redhat.com> In-Reply-To: <20190225152053.15976-1-kwolf@redhat.com> References: <20190225152053.15976-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 25 Feb 2019 15:21:43 +0000 (UTC) Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 23/71] block: fix bdrv_check_perm for non-tree subgraph X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, peter.maydell@linaro.org, qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" From: Vladimir Sementsov-Ogievskiy 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 Signed-off-by: Kevin Wolf --- 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; =20 + /* 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, Bloc= kReopenQueue *q, ret =3D bdrv_check_update_perm(c->bs, q, perm, shared, ignore_children= , errp); g_slist_free(ignore_children); =20 - return ret; + if (ret < 0) { + return ret; + } + + if (!c->has_backup_perm) { + c->has_backup_perm =3D true; + c->backup_perm =3D c->perm; + c->backup_shared_perm =3D 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 =3D perm; + c->shared_perm =3D shared; + + return 0; } =20 static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shar= ed) { uint64_t cumulative_perms, cumulative_shared_perms; =20 + c->has_backup_perm =3D false; + c->perm =3D perm; c->shared_perm =3D shared; =20 @@ -1971,6 +1990,12 @@ static void bdrv_child_set_perm(BdrvChild *c, uint64= _t perm, uint64_t shared) =20 static void bdrv_child_abort_perm_update(BdrvChild *c) { + if (c->has_backup_perm) { + c->perm =3D c->backup_perm; + c->shared_perm =3D c->backup_shared_perm; + c->has_backup_perm =3D false; + } + bdrv_abort_perm_update(c->bs); } =20 --=20 2.20.1