From nobody Thu May 9 01:02:42 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) client-ip=216.205.24.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1598284988; cv=none; d=zohomail.com; s=zohoarc; b=nCRI5+dy+x7QdqxN3rhtTPSCxucuDgnwAAYp8Uwjg2i4GaNoax0GFq/WcXT+mDYs4OYrgcPR1plpNI0D0AENW7eJLhMiQ9golxy+ScA/uh0QNwpXdBsJ/Xk9CfIyAu7+L0oN1k3Qf6GBiJvLH9OxWUE3j4BvvvED3H8ePLz0CUg= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1598284988; h=Content-Type:Content-Transfer-Encoding:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=KLzV8kzVB9n2Xgzk/89+aLML+v+Z+gTRiKYFDVVqE7c=; b=mohIQSEG3JX49kNIBL+JIdIJJbIT8T5hKe/Rxw65UXz5r3z9FIUoVeYMeNNalKYrVNa5exRqbdg/5qIDnyMHVumE4+IlpxDcJvZakgmLzioibcjTJ8de52UEm+B42+TPZ71+UzDsqnHGRtorUwGmHLCuFr6Ijwixu4n6jwmjmvc= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.zohomail.com with SMTPS id 1598284988085440.90432709344816; Mon, 24 Aug 2020 09:03:08 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-451-tQblXVMDO_qXHD5t99julA-1; Mon, 24 Aug 2020 12:03:03 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5C2E2801AC9; Mon, 24 Aug 2020 16:02:57 +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 EA3D9747A7; Mon, 24 Aug 2020 16:02:55 +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 935E6EC15; Mon, 24 Aug 2020 16:02:53 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 07OFq8OM029615 for ; Mon, 24 Aug 2020 11:52:08 -0400 Received: by smtp.corp.redhat.com (Postfix) id 566765D9E4; Mon, 24 Aug 2020 15:52:08 +0000 (UTC) Received: from speedmetal.redhat.com (unknown [10.40.208.11]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7E9725D9EF for ; Mon, 24 Aug 2020 15:52:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1598284986; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=KLzV8kzVB9n2Xgzk/89+aLML+v+Z+gTRiKYFDVVqE7c=; b=H7x7A8U96hActWKqNFagZ7/DCQPjeMU7gsjMM5wf4kGsaAKtk95woc3Ia7Mzt3areq74dy KYPY1HZvvIIVUVfS2rxa8PIWGQ6M5GydpuVvelxuzBq64j4uyH4tAJcmFWdtEsvqp0eHh6 TzEEFUYcuO3UC1fD/WU2wHxgGE0cLus= X-MC-Unique: tQblXVMDO_qXHD5t99julA-1 From: Peter Krempa To: libvir-list@redhat.com Subject: [PATCH] qemu: Move virQEMUFileOpenAs to qemu_domain.c Date: Mon, 24 Aug 2020 17:52:00 +0200 Message-Id: <2e10858a4d4f2e891e18613a955d86bb6b368987.1598284320.git.pkrempa@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com 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: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0.001 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" Commit 43620689794507308fbd3def6992a68ee2f8fa97 moved the function to util/virqemu.c which is compiled also on win32 and geteuid()/getegid() doesn't exist there. Move it to qemu_domain.c which is compiled only when the qemu driver is enabled. Originally I didn't want to put it here as qemu_domain.c is a code dump for helper functions but this is the least invasive fix. Signed-off-by: Peter Krempa Reviewed-by: Daniel P. Berrang=C3=A9 --- src/libvirt_private.syms | 1 - src/qemu/qemu_domain.c | 124 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 7 +++ src/util/virqemu.c | 130 --------------------------------------- src/util/virqemu.h | 7 --- 5 files changed, 131 insertions(+), 138 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d737e4d9e4..01c2e710cd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2939,7 +2939,6 @@ virQEMUBuildDriveCommandlineFromJSON; virQEMUBuildNetdevCommandlineFromJSON; virQEMUBuildObjectCommandlineFromJSON; virQEMUBuildQemuImgKeySecretOpts; -virQEMUFileOpenAs; # util/virrandom.h diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 715815f224..b321722f0e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10828,6 +10828,130 @@ qemuDomainDiskBlockJobIsSupported(virDomainObjPtr= vm, } +int +virQEMUFileOpenAs(uid_t fallback_uid, + gid_t fallback_gid, + bool dynamicOwnership, + const char *path, + int oflags, + bool *needUnlink) +{ + struct stat sb; + bool is_reg =3D true; + bool need_unlink =3D false; + unsigned int vfoflags =3D 0; + int fd =3D -1; + int path_shared =3D virFileIsSharedFS(path); + uid_t uid =3D geteuid(); + gid_t gid =3D getegid(); + + /* path might be a pre-existing block dev, in which case + * we need to skip the create step, and also avoid unlink + * in the failure case */ + if (oflags & O_CREAT) { + need_unlink =3D true; + + /* Don't force chown on network-shared FS + * as it is likely to fail. */ + if (path_shared <=3D 0 || dynamicOwnership) + vfoflags |=3D VIR_FILE_OPEN_FORCE_OWNER; + + if (stat(path, &sb) =3D=3D 0) { + /* It already exists, we don't want to delete it on error */ + need_unlink =3D false; + + is_reg =3D !!S_ISREG(sb.st_mode); + /* If the path is regular file which exists + * already and dynamic_ownership is off, we don't + * want to change its ownership, just open it as-is */ + if (is_reg && !dynamicOwnership) { + uid =3D sb.st_uid; + gid =3D sb.st_gid; + } + } + } + + /* First try creating the file as root */ + if (!is_reg) { + if ((fd =3D open(path, oflags & ~O_CREAT)) < 0) { + fd =3D -errno; + goto error; + } + } else { + if ((fd =3D virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR, uid, gi= d, + vfoflags | VIR_FILE_OPEN_NOFORK)) < 0) { + /* If we failed as root, and the error was permission-denied + (EACCES or EPERM), assume it's on a network-connected share + where root access is restricted (eg, root-squashed NFS). If= the + qemu user is non-root, just set a flag to + bypass security driver shenanigans, and retry the operation + after doing setuid to qemu user */ + if ((fd !=3D -EACCES && fd !=3D -EPERM) || fallback_uid =3D=3D= geteuid()) + goto error; + + /* On Linux we can also verify the FS-type of the directory. */ + switch (path_shared) { + case 1: + /* it was on a network share, so we'll continue + * as outlined above + */ + break; + + case -1: + virReportSystemError(-fd, oflags & O_CREAT + ? _("Failed to create file " + "'%s': couldn't determine fs = type") + : _("Failed to open file " + "'%s': couldn't determine fs = type"), + path); + goto cleanup; + + case 0: + default: + /* local file - log the error returned by virFileOpenA= s */ + goto error; + } + + /* If we created the file above, then we need to remove it; + * otherwise, the next attempt to create will fail. If the + * file had already existed before we got here, then we also + * don't want to delete it and allow the following to succeed + * or fail based on existing protections + */ + if (need_unlink) + unlink(path); + + /* Retry creating the file as qemu user */ + + /* Since we're passing different modes... */ + vfoflags |=3D VIR_FILE_OPEN_FORCE_MODE; + + if ((fd =3D virFileOpenAs(path, oflags, + S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP, + fallback_uid, fallback_gid, + vfoflags | VIR_FILE_OPEN_FORK)) < 0) { + virReportSystemError(-fd, oflags & O_CREAT + ? _("Error from child process creatin= g '%s'") + : _("Error from child process opening= '%s'"), + path); + goto cleanup; + } + } + } + cleanup: + if (needUnlink) + *needUnlink =3D need_unlink; + return fd; + + error: + virReportSystemError(-fd, oflags & O_CREAT + ? _("Failed to create file '%s'") + : _("Failed to open file '%s'"), + path); + goto cleanup; +} + + /** * qemuDomainOpenFile: * @driver: driver object diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 70fdb48317..e6d4acb8d4 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1024,6 +1024,13 @@ void qemuDomainRemoveInactiveJob(virQEMUDriverPtr dr= iver, void qemuDomainRemoveInactiveJobLocked(virQEMUDriverPtr driver, virDomainObjPtr vm); +int virQEMUFileOpenAs(uid_t fallback_uid, + gid_t fallback_gid, + bool dynamicOwnership, + const char *path, + int oflags, + bool *needUnlink); + int qemuDomainOpenFile(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/util/virqemu.c b/src/util/virqemu.c index e1c8673390..20cb09d878 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -22,18 +22,12 @@ #include -#include -#include -#include -#include - #include "virbuffer.h" #include "virerror.h" #include "virlog.h" #include "virqemu.h" #include "virstring.h" #include "viralloc.h" -#include "virfile.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -447,127 +441,3 @@ virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf, virBufferAddLit(buf, ","); } } - - -int -virQEMUFileOpenAs(uid_t fallback_uid, - gid_t fallback_gid, - bool dynamicOwnership, - const char *path, - int oflags, - bool *needUnlink) -{ - struct stat sb; - bool is_reg =3D true; - bool need_unlink =3D false; - unsigned int vfoflags =3D 0; - int fd =3D -1; - int path_shared =3D virFileIsSharedFS(path); - uid_t uid =3D geteuid(); - gid_t gid =3D getegid(); - - /* path might be a pre-existing block dev, in which case - * we need to skip the create step, and also avoid unlink - * in the failure case */ - if (oflags & O_CREAT) { - need_unlink =3D true; - - /* Don't force chown on network-shared FS - * as it is likely to fail. */ - if (path_shared <=3D 0 || dynamicOwnership) - vfoflags |=3D VIR_FILE_OPEN_FORCE_OWNER; - - if (stat(path, &sb) =3D=3D 0) { - /* It already exists, we don't want to delete it on error */ - need_unlink =3D false; - - is_reg =3D !!S_ISREG(sb.st_mode); - /* If the path is regular file which exists - * already and dynamic_ownership is off, we don't - * want to change its ownership, just open it as-is */ - if (is_reg && !dynamicOwnership) { - uid =3D sb.st_uid; - gid =3D sb.st_gid; - } - } - } - - /* First try creating the file as root */ - if (!is_reg) { - if ((fd =3D open(path, oflags & ~O_CREAT)) < 0) { - fd =3D -errno; - goto error; - } - } else { - if ((fd =3D virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR, uid, gi= d, - vfoflags | VIR_FILE_OPEN_NOFORK)) < 0) { - /* If we failed as root, and the error was permission-denied - (EACCES or EPERM), assume it's on a network-connected share - where root access is restricted (eg, root-squashed NFS). If= the - qemu user is non-root, just set a flag to - bypass security driver shenanigans, and retry the operation - after doing setuid to qemu user */ - if ((fd !=3D -EACCES && fd !=3D -EPERM) || fallback_uid =3D=3D= geteuid()) - goto error; - - /* On Linux we can also verify the FS-type of the directory. */ - switch (path_shared) { - case 1: - /* it was on a network share, so we'll continue - * as outlined above - */ - break; - - case -1: - virReportSystemError(-fd, oflags & O_CREAT - ? _("Failed to create file " - "'%s': couldn't determine fs = type") - : _("Failed to open file " - "'%s': couldn't determine fs = type"), - path); - goto cleanup; - - case 0: - default: - /* local file - log the error returned by virFileOpenA= s */ - goto error; - } - - /* If we created the file above, then we need to remove it; - * otherwise, the next attempt to create will fail. If the - * file had already existed before we got here, then we also - * don't want to delete it and allow the following to succeed - * or fail based on existing protections - */ - if (need_unlink) - unlink(path); - - /* Retry creating the file as qemu user */ - - /* Since we're passing different modes... */ - vfoflags |=3D VIR_FILE_OPEN_FORCE_MODE; - - if ((fd =3D virFileOpenAs(path, oflags, - S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP, - fallback_uid, fallback_gid, - vfoflags | VIR_FILE_OPEN_FORK)) < 0) { - virReportSystemError(-fd, oflags & O_CREAT - ? _("Error from child process creatin= g '%s'") - : _("Error from child process opening= '%s'"), - path); - goto cleanup; - } - } - } - cleanup: - if (needUnlink) - *needUnlink =3D need_unlink; - return fd; - - error: - virReportSystemError(-fd, oflags & O_CREAT - ? _("Failed to create file '%s'") - : _("Failed to open file '%s'"), - path); - goto cleanup; -} diff --git a/src/util/virqemu.h b/src/util/virqemu.h index e4e071f7c5..b1296cb657 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -63,10 +63,3 @@ void virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf, virStorageEncryptionInfoDefPtr enc, const char *alias) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); - -int virQEMUFileOpenAs(uid_t fallback_uid, - gid_t fallback_gid, - bool dynamicOwnership, - const char *path, - int oflags, - bool *needUnlink); --=20 2.26.2