From nobody Sun Apr 28 13:36:57 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 153752221725260.536670547921744; Fri, 21 Sep 2018 02:30:17 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AF94080462; Fri, 21 Sep 2018 09:30:14 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6FA52308BDB4; Fri, 21 Sep 2018 09:30:14 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 49C1C4A463; Fri, 21 Sep 2018 09:30:13 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w8L9U9pS022863 for ; Fri, 21 Sep 2018 05:30:09 -0400 Received: by smtp.corp.redhat.com (Postfix) id BA3B42010D7D; Fri, 21 Sep 2018 09:30:09 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.192]) by smtp.corp.redhat.com (Postfix) with ESMTP id 131702010D95; Fri, 21 Sep 2018 09:30:08 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Fri, 21 Sep 2018 11:29:56 +0200 Message-Id: <0585fe75bc9888cc9698b7816f4b2c98ee6bb79f.1537520636.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.25 X-loop: libvir-list@redhat.com Cc: bwalk@linux.ibm.com Subject: [libvirt] [PATCH 1/4] security: Grab a reference to virSecurityManager for transactions X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 21 Sep 2018 09:30:15 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" This shouldn't be needed per-se. Security manager shouldn't disappear during transactions - it's immutable. However, it doesn't hurt to grab a reference either - transaction code uses it after all. Signed-off-by: Michal Privoznik Reviewed-by: John Ferlan Reviewed-by: Marc Hartmayer --- src/security/security_dac.c | 5 +++-- src/security/security_selinux.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 2dbaf29ff5..5aea386e7c 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -141,6 +141,7 @@ virSecurityDACChownListFree(void *opaque) VIR_FREE(list->items[i]); } VIR_FREE(list->items); + virObjectUnref(list->manager); VIR_FREE(list); } =20 @@ -511,12 +512,12 @@ virSecurityDACTransactionStart(virSecurityManagerPtr = mgr) if (VIR_ALLOC(list) < 0) return -1; =20 - list->manager =3D mgr; + list->manager =3D virObjectRef(mgr); =20 if (virThreadLocalSet(&chownList, list) < 0) { virReportSystemError(errno, "%s", _("Unable to set thread local variable")); - VIR_FREE(list); + virSecurityDACChownListFree(list); return -1; } =20 diff --git a/src/security/security_selinux.c b/src/security/security_selinu= x.c index 056637e4cb..31e42afee7 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -156,6 +156,7 @@ virSecuritySELinuxContextListFree(void *opaque) for (i =3D 0; i < list->nItems; i++) virSecuritySELinuxContextItemFree(list->items[i]); =20 + virObjectUnref(list->manager); VIR_FREE(list); } =20 @@ -1054,12 +1055,12 @@ virSecuritySELinuxTransactionStart(virSecurityManag= erPtr mgr) if (VIR_ALLOC(list) < 0) return -1; =20 - list->manager =3D mgr; + list->manager =3D virObjectRef(mgr); =20 if (virThreadLocalSet(&contextList, list) < 0) { virReportSystemError(errno, "%s", _("Unable to set thread local variable")); - VIR_FREE(list); + virSecuritySELinuxContextListFree(list); return -1; } =20 --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Sun Apr 28 13:36:57 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1537522217256409.7531621338858; Fri, 21 Sep 2018 02:30:17 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AF6B9308124B; Fri, 21 Sep 2018 09:30:14 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7452F7FFD0; Fri, 21 Sep 2018 09:30:14 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 4AB9B4A465; Fri, 21 Sep 2018 09:30:13 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w8L9UAkr022873 for ; Fri, 21 Sep 2018 05:30:10 -0400 Received: by smtp.corp.redhat.com (Postfix) id BDBC52010D97; Fri, 21 Sep 2018 09:30:10 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.192]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1403D2010D95; Fri, 21 Sep 2018 09:30:09 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Fri, 21 Sep 2018 11:29:57 +0200 Message-Id: <40ad2b4a842a867c610194127571c3546fdb7a63.1537520636.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.25 X-loop: libvir-list@redhat.com Cc: bwalk@linux.ibm.com Subject: [libvirt] [PATCH 2/4] virNetSocket: Be more safe with fork() around virNetSocketDupFD() X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Fri, 21 Sep 2018 09:30:16 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" If there was a caller which would dup the client FD without CLOEXEC flag and later decided to change the flag it wouldn't be safe to do because fork() might have had occurred meantime. Switch to the other pattern - always dup FD with the flag set and let callers clear the flag if they need to do so. Signed-off-by: Michal Privoznik --- src/libxl/libxl_migration.c | 4 ++-- src/qemu/qemu_migration.c | 2 +- src/rpc/virnetclient.c | 10 +++++++++- src/rpc/virnetsocket.c | 7 ++----- src/rpc/virnetsocket.h | 2 +- 5 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index fc7ccb53d0..5eb8eb1203 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -310,7 +310,7 @@ libxlMigrateDstReceive(virNetSocketPtr sock, } VIR_DEBUG("Accepted migration connection." " Spawning thread to process migration data"); - recvfd =3D virNetSocketDupFD(client_sock, true); + recvfd =3D virNetSocketDupFD(client_sock); virObjectUnref(client_sock); =20 /* @@ -1254,7 +1254,7 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivatePtr = driver, goto cleanup; } =20 - sockfd =3D virNetSocketDupFD(sock, true); + sockfd =3D virNetSocketDupFD(sock); virObjectUnref(sock); =20 if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockStat= e) < 0) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 825a9d399b..129be0a11a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3303,7 +3303,7 @@ qemuMigrationSrcConnect(virQEMUDriverPtr driver, if (virNetSocketNewConnectTCP(host, port, AF_UNSPEC, &sock) =3D=3D 0) { - spec->dest.fd.qemu =3D virNetSocketDupFD(sock, true); + spec->dest.fd.qemu =3D virNetSocketDupFD(sock); virObjectUnref(sock); } if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0= || diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b4d8fb2187..40ed3fde6d 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -715,8 +715,16 @@ int virNetClientGetFD(virNetClientPtr client) int virNetClientDupFD(virNetClientPtr client, bool cloexec) { int fd; + virObjectLock(client); - fd =3D virNetSocketDupFD(client->sock, cloexec); + + fd =3D virNetSocketDupFD(client->sock); + if (!cloexec && fd >=3D 0 && virSetInherit(fd, true)) { + virReportSystemError(errno, "%s", + _("Cannot disable close-on-exec flag")); + VIR_FORCE_CLOSE(fd); + } + virObjectUnlock(client); return fd; } diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 55de3b2aad..27ffa23bcd 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -1372,14 +1372,11 @@ int virNetSocketGetFD(virNetSocketPtr sock) } =20 =20 -int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec) +int virNetSocketDupFD(virNetSocketPtr sock) { int fd; =20 - if (cloexec) - fd =3D fcntl(sock->fd, F_DUPFD_CLOEXEC, 0); - else - fd =3D dup(sock->fd); + fd =3D fcntl(sock->fd, F_DUPFD_CLOEXEC, 0); if (fd < 0) { virReportSystemError(errno, "%s", _("Unable to copy socket file handle")); diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index de795af917..e6bac27566 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -124,7 +124,7 @@ virNetSocketPtr virNetSocketNewPostExecRestart(virJSONV= aluePtr object); virJSONValuePtr virNetSocketPreExecRestart(virNetSocketPtr sock); =20 int virNetSocketGetFD(virNetSocketPtr sock); -int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec); +int virNetSocketDupFD(virNetSocketPtr sock); =20 bool virNetSocketIsLocal(virNetSocketPtr sock); =20 --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Sun Apr 28 13:36:57 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1537522233198651.9914556931211; Fri, 21 Sep 2018 02:30:33 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 340D4309B6C4; Fri, 21 Sep 2018 09:30:30 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0786A806BC; Fri, 21 Sep 2018 09:30:30 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id B2702181A12F; Fri, 21 Sep 2018 09:30:29 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w8L9UBdX022878 for ; Fri, 21 Sep 2018 05:30:11 -0400 Received: by smtp.corp.redhat.com (Postfix) id BD3C52010D7D; Fri, 21 Sep 2018 09:30:11 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.192]) by smtp.corp.redhat.com (Postfix) with ESMTP id 16CC62010D95; Fri, 21 Sep 2018 09:30:10 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Fri, 21 Sep 2018 11:29:58 +0200 Message-Id: <2293db84dc47d411373bed6f58106ac37796b5c9.1537520636.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.25 X-loop: libvir-list@redhat.com Cc: bwalk@linux.ibm.com Subject: [libvirt] [PATCH 3/4] virLockManagerLockDaemonAcquire: Duplicate client FD with CLOEXEC flag X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Fri, 21 Sep 2018 09:30:31 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" There is one caller (virSecurityManagerMetadataLock) which duplicates the connection FD and wants to have the flag set. However, trying to set the flag after dup() is not safe as another thread might fork() meanwhile. Therefore, switch to duplicating with the flag set and only let callers refine this later. Signed-off-by: Michal Privoznik --- src/locking/domain_lock.c | 18 ++++++++++++++++++ src/locking/lock_driver_lockd.c | 2 +- src/rpc/virnetclient.c | 9 +-------- src/rpc/virnetclient.h | 2 +- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index 705b132457..db20fa86a3 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -188,6 +188,24 @@ int virDomainLockProcessStart(virLockManagerPluginPtr = plugin, ret =3D virLockManagerAcquire(lock, NULL, flags, dom->def->onLockFailure, fd); =20 + if (ret >=3D 0 && fd && *fd >=3D 0 && virSetInherit(*fd, true) < 0) { + int saved_errno =3D errno; + virErrorPtr origerr; + + virErrorPreserveLast(&origerr); + + if (virLockManagerRelease(lock, NULL, 0) < 0) + VIR_WARN("Unable to release acquired resourced in cleanup path= "); + + virErrorRestore(&origerr); + errno =3D saved_errno; + + virReportSystemError(errno, "%s", + _("Cannot disable close-on-exec flag")); + + ret =3D -1; + } + virLockManagerFree(lock); =20 return ret; diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lock= d.c index 0c672b05b1..9b1943daa6 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -796,7 +796,7 @@ static int virLockManagerLockDaemonAcquire(virLockManag= erPtr lock, goto cleanup; =20 if (fd && - (*fd =3D virNetClientDupFD(client, false)) < 0) + (*fd =3D virNetClientDupFD(client)) < 0) goto cleanup; =20 if (!(flags & VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY)) { diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 40ed3fde6d..6b0ddbeaad 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -712,19 +712,12 @@ int virNetClientGetFD(virNetClientPtr client) } =20 =20 -int virNetClientDupFD(virNetClientPtr client, bool cloexec) +int virNetClientDupFD(virNetClientPtr client) { int fd; =20 virObjectLock(client); - fd =3D virNetSocketDupFD(client->sock); - if (!cloexec && fd >=3D 0 && virSetInherit(fd, true)) { - virReportSystemError(errno, "%s", - _("Cannot disable close-on-exec flag")); - VIR_FORCE_CLOSE(fd); - } - virObjectUnlock(client); return fd; } diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 9cf32091f5..3702f7fe5a 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -95,7 +95,7 @@ void virNetClientSetCloseCallback(virNetClientPtr client, virFreeCallback ff); =20 int virNetClientGetFD(virNetClientPtr client); -int virNetClientDupFD(virNetClientPtr client, bool cloexec); +int virNetClientDupFD(virNetClientPtr client); =20 bool virNetClientHasPassFD(virNetClientPtr client); =20 --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Sun Apr 28 13:36:57 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1537522232671653.7747177040959; Fri, 21 Sep 2018 02:30:32 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4BEEA30001D3; Fri, 21 Sep 2018 09:30:30 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 18305309138F; Fri, 21 Sep 2018 09:30:30 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id BE04B181A130; Fri, 21 Sep 2018 09:30:29 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w8L9UCWx022885 for ; Fri, 21 Sep 2018 05:30:12 -0400 Received: by smtp.corp.redhat.com (Postfix) id C01152010D97; Fri, 21 Sep 2018 09:30:12 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.192]) by smtp.corp.redhat.com (Postfix) with ESMTP id 15A5E2010D7D; Fri, 21 Sep 2018 09:30:11 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Fri, 21 Sep 2018 11:29:59 +0200 Message-Id: <28329b4885a6d9ccfe5e68c748a825ebc8ed10fe.1537520636.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.25 X-loop: libvir-list@redhat.com Cc: bwalk@linux.ibm.com Subject: [libvirt] [PATCH 4/4] security: Always spawn process for transactions X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Fri, 21 Sep 2018 09:30:31 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" There is this latent bug which can result in virtlockd killing libvirtd. The problem is when the following steps occur: Parent | Child ------------------------------------------------------+---------------- 1) virSecurityManagerMetadataLock(path); | This results in connection FD to virtlockd to be | dup()-ed and saved for later use. | | 2) Another thread calling fork(); | This results in the FD from step 1) to be cloned | | 3) virSecurityManagerMetadataUnlock(path); | Here, the FD is closed, but the connection is | still kept open because child process has | duplicated FD | | 4) virSecurityManagerMetadataLock(differentPath); | Again, new connection is opened, FD is dup()-ed | | 5) | exec() or exit() Step 5) results in closing the connection from 1) to be closed, finally. However, at this point virtlockd looks at its internal records if PID from 1) does not still own any resources. Well it does - in step 4) it locked differentPath. This results in virtlockd killing libvirtd. Note that this happens because we do some relabelling even before fork() at which point vm->pid is still -1. When this value is passed down to transaction code it simply runs the transaction in parent process (=3Dlibvirtd), which means the owner of metadata locks is the parent process. Signed-off-by: Michal Privoznik Signed-off-by: Michal Privoznik --- src/security/security_dac.c | 12 ++++++------ src/security/security_selinux.c | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 5aea386e7c..876cca0f2f 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -561,12 +561,12 @@ virSecurityDACTransactionCommit(virSecurityManagerPtr= mgr ATTRIBUTE_UNUSED, goto cleanup; } =20 - if ((pid =3D=3D -1 && - virSecurityDACTransactionRun(pid, list) < 0) || - (pid !=3D -1 && - virProcessRunInMountNamespace(pid, - virSecurityDACTransactionRun, - list) < 0)) + if (pid =3D=3D -1) + pid =3D getpid(); + + if (virProcessRunInMountNamespace(pid, + virSecurityDACTransactionRun, + list) < 0) goto cleanup; =20 ret =3D 0; diff --git a/src/security/security_selinux.c b/src/security/security_selinu= x.c index 31e42afee7..1e23d776ff 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1102,12 +1102,12 @@ virSecuritySELinuxTransactionCommit(virSecurityMana= gerPtr mgr ATTRIBUTE_UNUSED, goto cleanup; } =20 - if ((pid =3D=3D -1 && - virSecuritySELinuxTransactionRun(pid, list) < 0) || - (pid !=3D -1 && - virProcessRunInMountNamespace(pid, - virSecuritySELinuxTransactionRun, - list) < 0)) + if (pid =3D=3D -1) + pid =3D getpid(); + + if (virProcessRunInMountNamespace(pid, + virSecuritySELinuxTransactionRun, + list) < 0) goto cleanup; =20 ret =3D 0; --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Sun Apr 28 13:36:57 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1537860920967505.9752331703371; Tue, 25 Sep 2018 00:35:20 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E80CFCF27; Tue, 25 Sep 2018 07:35:17 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 038185DD78; Tue, 25 Sep 2018 07:35:16 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 613D2181A12E; Tue, 25 Sep 2018 07:35:11 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w8P7Z9VY013854 for ; Tue, 25 Sep 2018 03:35:09 -0400 Received: by smtp.corp.redhat.com (Postfix) id 508412010D7F; Tue, 25 Sep 2018 07:35:09 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.192]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9F5192010D0A; Tue, 25 Sep 2018 07:35:05 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Tue, 25 Sep 2018 09:34:50 +0200 Message-Id: <324b73ff7bf86ba5e42053a76d2b2b10ca137319.1537860836.git.mprivozn@redhat.com> In-Reply-To: References: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.25 X-loop: libvir-list@redhat.com Cc: bwalk@linux.ibm.com Subject: [libvirt] [PATCH 5/4] security: Don't try to lock NULL paths X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 25 Sep 2018 07:35:19 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" It may happen that in the list of paths/disk sources to relabel there is a disk source. If that is the case, the path is NULL. In that case, we shouldn't try to lock the path. It's likely a network disk anyway and therefore there is nothing to lock. Signed-off-by: Michal Privoznik Reviewed-by: Erik Skultety --- src/security/security_dac.c | 3 ++- src/security/security_selinux.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 876cca0f2f..04168feb3d 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -216,7 +216,8 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED, for (i =3D 0; i < list->nItems; i++) { const char *p =3D list->items[i]->path; =20 - if (virFileIsDir(p)) + if (!p || + virFileIsDir(p)) continue; =20 VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p); diff --git a/src/security/security_selinux.c b/src/security/security_selinu= x.c index 3c847d8dcb..3adbeada14 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -227,7 +227,8 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UN= USED, for (i =3D 0; i < list->nItems; i++) { const char *p =3D list->items[i]->path; =20 - if (virFileIsDir(p)) + if (!p || + virFileIsDir(p)) continue; =20 VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p); --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list