From nobody Tue Apr 8 16:37:08 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.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 208.118.235.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 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535726161602588.330565537334; Fri, 31 Aug 2018 07:36:01 -0700 (PDT) Received: from localhost ([::1]:53991 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkWm-0007BV-De for importer@patchew.org; Fri, 31 Aug 2018 10:36:00 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50667) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQZ-0001A5-5g for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkMA-0000kY-0T for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:08 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:50280 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkM4-0000ip-CM; Fri, 31 Aug 2018 10:24:58 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0BDAB80825A9; Fri, 31 Aug 2018 14:24:51 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B162663F57; Fri, 31 Aug 2018 14:24:50 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:36 +0200 Message-Id: <20180831142446.22264-2-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 31 Aug 2018 14:24:51 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 31 Aug 2018 14:24:51 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 01/11] file-posix: Skip effectiveless OFD lock operations 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: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Fam Zheng If we know we've already locked the bytes, don't do it again; similarly don't unlock a byte if we haven't locked it. This doesn't change the behavior, but fixes a corner case explained below. Libvirt had an error handling bug that an image can get its (ownership, file mode, SELinux) permissions changed (RHBZ 1584982) by mistake behind QEMU. Specifically, an image in use by Libvirt VM has: $ ls -lhZ b.img -rw-r--r--. qemu qemu system_u:object_r:svirt_image_t:s0:c600,c690 b.img Trying to attach it a second time won't work because of image locking. And after the error, it becomes: $ ls -lhZ b.img -rw-r--r--. root root system_u:object_r:virt_image_t:s0 b.img Then, we won't be able to do OFD lock operations with the existing fd. In other words, the code such as in blk_detach_dev: blk_set_perm(blk, 0, BLK_PERM_ALL, &error_abort); can abort() QEMU, out of environmental changes. This patch is an easy fix to this and the change is regardlessly reasonable, so do it. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/file-posix.c | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index fe83cbf0eb..73ae00c8c5 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -680,23 +680,42 @@ typedef enum { * file; if @unlock =3D=3D true, also unlock the unneeded bytes. * @shared_perm_lock_bits is the mask of all permissions that are NOT shar= ed. */ -static int raw_apply_lock_bytes(int fd, +static int raw_apply_lock_bytes(BDRVRawState *s, int fd, uint64_t perm_lock_bits, uint64_t shared_perm_lock_bits, bool unlock, Error **errp) { int ret; int i; + uint64_t locked_perm, locked_shared_perm; + + if (s) { + locked_perm =3D s->perm; + locked_shared_perm =3D ~s->shared_perm & BLK_PERM_ALL; + } else { + /* + * We don't have the previous bits, just lock/unlock for each of t= he + * requested bits. + */ + if (unlock) { + locked_perm =3D BLK_PERM_ALL; + locked_shared_perm =3D BLK_PERM_ALL; + } else { + locked_perm =3D 0; + locked_shared_perm =3D 0; + } + } =20 PERM_FOREACH(i) { int off =3D RAW_LOCK_PERM_BASE + i; - if (perm_lock_bits & (1ULL << i)) { + uint64_t bit =3D (1ULL << i); + if ((perm_lock_bits & bit) && !(locked_perm & bit)) { ret =3D qemu_lock_fd(fd, off, 1, false); if (ret) { error_setg(errp, "Failed to lock byte %d", off); return ret; } - } else if (unlock) { + } else if (unlock && (locked_perm & bit) && !(perm_lock_bits & bit= )) { ret =3D qemu_unlock_fd(fd, off, 1); if (ret) { error_setg(errp, "Failed to unlock byte %d", off); @@ -706,13 +725,15 @@ static int raw_apply_lock_bytes(int fd, } PERM_FOREACH(i) { int off =3D RAW_LOCK_SHARED_BASE + i; - if (shared_perm_lock_bits & (1ULL << i)) { + uint64_t bit =3D (1ULL << i); + if ((shared_perm_lock_bits & bit) && !(locked_shared_perm & bit)) { ret =3D qemu_lock_fd(fd, off, 1, false); if (ret) { error_setg(errp, "Failed to lock byte %d", off); return ret; } - } else if (unlock) { + } else if (unlock && (locked_shared_perm & bit) && + !(shared_perm_lock_bits & bit)) { ret =3D qemu_unlock_fd(fd, off, 1); if (ret) { error_setg(errp, "Failed to unlock byte %d", off); @@ -788,7 +809,7 @@ static int raw_handle_perm_lock(BlockDriverState *bs, =20 switch (op) { case RAW_PL_PREPARE: - ret =3D raw_apply_lock_bytes(s->lock_fd, s->perm | new_perm, + ret =3D raw_apply_lock_bytes(s, s->lock_fd, s->perm | new_perm, ~s->shared_perm | ~new_shared, false, errp); if (!ret) { @@ -800,7 +821,7 @@ static int raw_handle_perm_lock(BlockDriverState *bs, op =3D RAW_PL_ABORT; /* fall through to unlock bytes. */ case RAW_PL_ABORT: - raw_apply_lock_bytes(s->lock_fd, s->perm, ~s->shared_perm, + raw_apply_lock_bytes(s, s->lock_fd, s->perm, ~s->shared_perm, true, &local_err); if (local_err) { /* Theoretically the above call only unlocks bytes and it cann= ot @@ -810,7 +831,7 @@ static int raw_handle_perm_lock(BlockDriverState *bs, } break; case RAW_PL_COMMIT: - raw_apply_lock_bytes(s->lock_fd, new_perm, ~new_shared, + raw_apply_lock_bytes(s, s->lock_fd, new_perm, ~new_shared, true, &local_err); if (local_err) { /* Theoretically the above call only unlocks bytes and it cann= ot @@ -2209,7 +2230,7 @@ raw_co_create(BlockdevCreateOptions *options, Error *= *errp) shared =3D BLK_PERM_ALL & ~BLK_PERM_RESIZE; =20 /* Step one: Take locks */ - result =3D raw_apply_lock_bytes(fd, perm, ~shared, false, errp); + result =3D raw_apply_lock_bytes(NULL, fd, perm, ~shared, false, errp); if (result < 0) { goto out_close; } @@ -2250,7 +2271,7 @@ raw_co_create(BlockdevCreateOptions *options, Error *= *errp) } =20 out_unlock: - raw_apply_lock_bytes(fd, 0, 0, true, &local_err); + raw_apply_lock_bytes(NULL, fd, 0, 0, true, &local_err); if (local_err) { /* The above call should not fail, and if it does, that does * not mean the whole creation operation has failed. So --=20 2.17.1